Merge pull request #20384 from keszybz/udev-code-modernization

Various minor refactorings (basic/log, udev, cryptsetup-tokens)

src/basic/log.c

@@ -476,7 +476,7 @@ static int write_to_syslog(
                 IOVEC_MAKE_STRING(header_pid),
                 IOVEC_MAKE_STRING(buffer),
         };
-        struct msghdr msghdr = {
+        const struct msghdr msghdr = {
                 .msg_iov = iovec,
                 .msg_iovlen = ELEMENTSOF(iovec),
         };
@@ -608,23 +608,24 @@ static int write_to_journal(
                 const char *buffer) {
 
         char header[LINE_MAX];
-        struct iovec iovec[4] = {};
-        struct msghdr mh = {};
 
         if (journal_fd < 0)
                 return 0;
 
         log_do_header(header, sizeof(header), level, error, file, line, func, object_field, object, extra_field, extra);
 
-        iovec[0] = IOVEC_MAKE_STRING(header);
+        struct iovec iovec[4] = {
-        iovec[1] = IOVEC_MAKE_STRING("MESSAGE=");
+                IOVEC_MAKE_STRING(header),
-        iovec[2] = IOVEC_MAKE_STRING(buffer);
+                IOVEC_MAKE_STRING("MESSAGE="),
-        iovec[3] = IOVEC_MAKE_STRING("\n");
+                IOVEC_MAKE_STRING(buffer),
+                IOVEC_MAKE_STRING("\n"),
+        };
+        const struct msghdr msghdr = {
+                .msg_iov = iovec,
+                .msg_iovlen = ELEMENTSOF(iovec),
+        };
 
-        mh.msg_iov = iovec;
+        if (sendmsg(journal_fd, &msghdr, MSG_NOSIGNAL) < 0)
-        mh.msg_iovlen = ELEMENTSOF(iovec);
-
-        if (sendmsg(journal_fd, &mh, MSG_NOSIGNAL) < 0)
                 return -errno;
 
         return 1;
@@ -918,11 +919,8 @@ int log_format_iovec(
                 VA_FORMAT_ADVANCE(format, ap);
 
                 iovec[(*n)++] = IOVEC_MAKE_STRING(m);
-
+                if (newline_separator)
-                if (newline_separator) {
+                        iovec[(*n)++] = IOVEC_MAKE((char *)&nl, 1);
-                        iovec[*n] = IOVEC_MAKE((char *)&nl, 1);
-                        (*n)++;
-                }
 
                 format = va_arg(ap, char *);
         }
@@ -959,12 +957,9 @@ int log_struct_internal(
 
                 if (journal_fd >= 0) {
                         char header[LINE_MAX];
-                        struct iovec iovec[17] = {};
+                        struct iovec iovec[17];
                         size_t n = 0;
                         int r;
-                        struct msghdr mh = {
-                                .msg_iov = iovec,
-                        };
                         bool fallback = false;
 
                         /* If the journal is available do structured logging.
@@ -977,8 +972,12 @@ int log_struct_internal(
                         if (r < 0)
                                 fallback = true;
                         else {
-                                mh.msg_iovlen = n;
+                                const struct msghdr msghdr = {
-                                (void) sendmsg(journal_fd, &mh, MSG_NOSIGNAL);
+                                        .msg_iov = iovec,
+                                        .msg_iovlen = n,
+                                };
+
+                                (void) sendmsg(journal_fd, &msghdr, MSG_NOSIGNAL);
                         }
 
                         va_end(ap);
@@ -1037,8 +1036,6 @@ int log_struct_iovec_internal(
                 size_t n_input_iovec) {
 
         PROTECT_ERRNO;
-        size_t i;
-        char *m;
 
         if (_likely_(LOG_PRI(level) > log_max_level) ||
             log_target == LOG_TARGET_NULL)
@@ -1052,38 +1049,37 @@ int log_struct_iovec_internal(
                                LOG_TARGET_JOURNAL) &&
             journal_fd >= 0) {
 
-                struct iovec iovec[1 + n_input_iovec*2];
                 char header[LINE_MAX];
-                struct msghdr mh = {
-                        .msg_iov = iovec,
-                        .msg_iovlen = 1 + n_input_iovec*2,
-                };
-
                 log_do_header(header, sizeof(header), level, error, file, line, func, NULL, NULL, NULL, NULL);
-                iovec[0] = IOVEC_MAKE_STRING(header);
 
-                for (i = 0; i < n_input_iovec; i++) {
+                struct iovec iovec[1 + n_input_iovec*2];
+                iovec[0] = IOVEC_MAKE_STRING(header);
+                for (size_t i = 0; i < n_input_iovec; i++) {
                         iovec[1+i*2] = input_iovec[i];
                         iovec[1+i*2+1] = IOVEC_MAKE_STRING("\n");
                 }
 
-                if (sendmsg(journal_fd, &mh, MSG_NOSIGNAL) >= 0)
+                const struct msghdr msghdr = {
+                        .msg_iov = iovec,
+                        .msg_iovlen = 1 + n_input_iovec*2,
+                };
+
+                if (sendmsg(journal_fd, &msghdr, MSG_NOSIGNAL) >= 0)
                         return -ERRNO_VALUE(error);
         }
 
-        for (i = 0; i < n_input_iovec; i++)
+        for (size_t i = 0; i < n_input_iovec; i++)
-                if (memory_startswith(input_iovec[i].iov_base, input_iovec[i].iov_len, "MESSAGE="))
+                if (memory_startswith(input_iovec[i].iov_base, input_iovec[i].iov_len, "MESSAGE=")) {
-                        break;
+                        char *m = strndupa(input_iovec[i].iov_base + STRLEN("MESSAGE="),
-
-        if (_unlikely_(i >= n_input_iovec)) /* Couldn't find MESSAGE=? */
-                return -ERRNO_VALUE(error);
-
-        m = strndupa(input_iovec[i].iov_base + STRLEN("MESSAGE="),
                                            input_iovec[i].iov_len - STRLEN("MESSAGE="));
 
                         return log_dispatch_internal(level, error, file, line, func, NULL, NULL, NULL, NULL, m);
                 }
 
+        /* Couldn't find MESSAGE=. */
+        return -ERRNO_VALUE(error);
+}
+
 int log_set_target_from_string(const char *e) {
         LogTarget t;
 

src/cryptsetup/cryptsetup-tokens/cryptsetup-token-systemd-tpm2.c

@@ -134,7 +134,7 @@ _public_ void cryptsetup_token_dump(
                 const char *json /* validated 'systemd-tpm2' token if cryptsetup_token_validate is defined */) {
 
         int r;
-        uint32_t i, pcr_mask;
+        uint32_t pcr_mask;
         uint16_t pcr_bank;
         size_t decoded_blob_size;
         _cleanup_free_ char *base64_blob = NULL, *hex_policy_hash = NULL,
@@ -147,7 +147,7 @@ _public_ void cryptsetup_token_dump(
         if (r < 0)
                 return (void) crypt_log_debug_errno(cd, r, "Failed to parse " TOKEN_NAME " metadata: %m.");
 
-        for (i = 0; i < TPM2_PCRS_MAX; i++) {
+        for (uint32_t i = 0; i < TPM2_PCRS_MAX; i++) {
                 if ((pcr_mask & (UINT32_C(1) << i)) &&
                     ((r = strextendf_with_separator(&pcrs_str, ", ", "%" PRIu32, i)) < 0))
                         return (void) crypt_log_debug_errno(cd, r, "Can not dump " TOKEN_NAME " content: %m");

src/test/test-log.c

@@ -66,11 +66,11 @@ static void test_log_syntax(void) {
 }
 
 int main(int argc, char* argv[]) {
-        int target;
-
         test_file();
 
-        for (target = 0; target < _LOG_TARGET_MAX; target++) {
+        assert_se(log_info_errno(SYNTHETIC_ERRNO(EUCLEAN), "foo") == -EUCLEAN);
+
+        for (int target = 0; target < _LOG_TARGET_MAX; target++) {
                 log_set_target(target);
                 log_open();
 
@@ -79,7 +79,5 @@ int main(int argc, char* argv[]) {
                 test_log_syntax();
         }
 
-        assert_se(log_info_errno(SYNTHETIC_ERRNO(EUCLEAN), "foo") == -EUCLEAN);
-
         return 0;
 }

src/udev/udev-builtin-blkid.c

@@ -116,8 +116,7 @@ static int find_gpt_root(sd_device *dev, blkid_probe pr, bool test) {
 
         _cleanup_free_ char *root_id = NULL, *root_label = NULL;
         bool found_esp = false;
-        blkid_partlist pl;
+        int r;
-        int i, nvals, r;
 
         assert(pr);
 
@@ -126,12 +125,12 @@ static int find_gpt_root(sd_device *dev, blkid_probe pr, bool test) {
          * disk, and add a property indicating its partition UUID. */
 
         errno = 0;
-        pl = blkid_probe_get_partitions(pr);
+        blkid_partlist pl = blkid_probe_get_partitions(pr);
         if (!pl)
                 return errno_or_else(ENOMEM);
 
-        nvals = blkid_partlist_numof_partitions(pl);
+        int nvals = blkid_partlist_numof_partitions(pl);
-        for (i = 0; i < nvals; i++) {
+        for (int i = 0; i < nvals; i++) {
                 blkid_partition pp;
                 const char *stype, *sid, *label;
                 sd_id128_t type;
@@ -240,7 +239,7 @@ static int builtin_blkid(sd_device *dev, int argc, char *argv[], bool test) {
         bool noraid = false, is_gpt = false;
         _cleanup_close_ int fd = -1;
         int64_t offset = 0;
-        int nvals, i, r;
+        int r;
 
         static const struct option options[] = {
                 { "offset", required_argument, NULL, 'o' },
@@ -325,11 +324,11 @@ static int builtin_blkid(sd_device *dev, int argc, char *argv[], bool test) {
         (void) sd_device_get_property_value(dev, "ID_PART_GPT_AUTO_ROOT_UUID", &root_partition);
 
         errno = 0;
-        nvals = blkid_probe_numof_values(pr);
+        int nvals = blkid_probe_numof_values(pr);
         if (nvals < 0)
                 return log_device_debug_errno(dev, errno_or_else(ENOMEM), "Failed to get number of probed values: %m");
 
-        for (i = 0; i < nvals; i++) {
+        for (int i = 0; i < nvals; i++) {
                 if (blkid_probe_get_value(pr, i, &name, &data, NULL) < 0)
                         continue;
 

src/udev/udev-builtin-input_id.c

@@ -92,7 +92,7 @@ static void get_cap_mask(sd_device *pdev, const char* attr,
 
         memzero(bitmask, bitmask_size);
         i = 0;
-        while ((word = strrchr(text, ' ')) != NULL) {
+        while ((word = strrchr(text, ' '))) {
                 r = safe_atolu_full(word+1, 16, &val);
                 if (r < 0)
                         log_device_debug_errno(pdev, r, "Ignoring %s block which failed to parse: %m", attr);
@@ -101,7 +101,7 @@ static void get_cap_mask(sd_device *pdev, const char* attr,
                 else
                         log_device_debug(pdev, "Ignoring %s block %lX which is larger than maximum size", attr, val);
                 *word = '\0';
-                ++i;
+                i++;
         }
         r = safe_atolu_full(text, 16, &val);
         if (r < 0)
@@ -120,9 +120,9 @@ static void get_cap_mask(sd_device *pdev, const char* attr,
                 /* skip over leading zeros */
                 while (bitmask[val-1] == 0 && val > 0)
                         --val;
-                for (i = 0; i < val; ++i) {
+                for (unsigned long j = 0; j < val; j++) {
                         DISABLE_WARNING_FORMAT_NONLITERAL;
-                        log_device_debug(pdev, text, i * BITS_PER_LONG, bitmask[i]);
+                        log_device_debug(pdev, text, j * BITS_PER_LONG, bitmask[j]);
                         REENABLE_WARNING;
                 }
         }
@@ -153,7 +153,6 @@ static bool test_pointers(sd_device *dev,
                           const unsigned long* bitmask_rel,
                           const unsigned long* bitmask_props,
                           bool test) {
-        int button, axis;
         bool has_abs_coordinates = false;
         bool has_rel_coordinates = false;
         bool has_mt_coordinates = false;
@@ -193,7 +192,7 @@ static bool test_pointers(sd_device *dev,
         has_stylus = test_bit(BTN_STYLUS, bitmask_key);
         has_pen = test_bit(BTN_TOOL_PEN, bitmask_key);
         finger_but_no_pen = test_bit(BTN_TOOL_FINGER, bitmask_key) && !test_bit(BTN_TOOL_PEN, bitmask_key);
-        for (button = BTN_MOUSE; button < BTN_JOYSTICK && !has_mouse_button; button++)
+        for (int button = BTN_MOUSE; button < BTN_JOYSTICK && !has_mouse_button; button++)
                 has_mouse_button = test_bit(button, bitmask_key);
         has_rel_coordinates = test_bit(EV_REL, bitmask_ev) && test_bit(REL_X, bitmask_rel) && test_bit(REL_Y, bitmask_rel);
         has_mt_coordinates = test_bit(ABS_MT_POSITION_X, bitmask_abs) && test_bit(ABS_MT_POSITION_Y, bitmask_abs);
@@ -214,14 +213,14 @@ static bool test_pointers(sd_device *dev,
          * Catz Mad Catz M.M.O.TE). Skip those.
          */
         if (!test_bit(BTN_JOYSTICK - 1, bitmask_key)) {
-                for (button = BTN_JOYSTICK; button < BTN_DIGI && !has_joystick_axes_or_buttons; button++)
+                for (int button = BTN_JOYSTICK; button < BTN_DIGI && !has_joystick_axes_or_buttons; button++)
                         has_joystick_axes_or_buttons = test_bit(button, bitmask_key);
-                for (button = BTN_TRIGGER_HAPPY1; button <= BTN_TRIGGER_HAPPY40 && !has_joystick_axes_or_buttons; button++)
+                for (int button = BTN_TRIGGER_HAPPY1; button <= BTN_TRIGGER_HAPPY40 && !has_joystick_axes_or_buttons; button++)
                         has_joystick_axes_or_buttons = test_bit(button, bitmask_key);
-                for (button = BTN_DPAD_UP; button <= BTN_DPAD_RIGHT && !has_joystick_axes_or_buttons; button++)
+                for (int button = BTN_DPAD_UP; button <= BTN_DPAD_RIGHT && !has_joystick_axes_or_buttons; button++)
                         has_joystick_axes_or_buttons = test_bit(button, bitmask_key);
         }
-        for (axis = ABS_RX; axis < ABS_PRESSURE && !has_joystick_axes_or_buttons; axis++)
+        for (int axis = ABS_RX; axis < ABS_PRESSURE && !has_joystick_axes_or_buttons; axis++)
                 has_joystick_axes_or_buttons = test_bit(axis, bitmask_abs);
 
         if (has_abs_coordinates) {
@@ -285,10 +284,8 @@ static bool test_key(sd_device *dev,
                      const unsigned long* bitmask_ev,
                      const unsigned long* bitmask_key,
                      bool test) {
-        unsigned i;
+
-        unsigned long found;
+        bool found = false;
-        unsigned long mask;
-        bool ret = false;
 
         /* do we have any KEY_* capability? */
         if (!test_bit(EV_KEY, bitmask_ev)) {
@@ -297,39 +294,32 @@ static bool test_key(sd_device *dev,
         }
 
         /* only consider KEY_* here, not BTN_* */
-        found = 0;
+        for (size_t i = 0; i < BTN_MISC/BITS_PER_LONG && !found; i++) {
-        for (i = 0; i < BTN_MISC/BITS_PER_LONG; ++i) {
+                if (bitmask_key[i])
-                found |= bitmask_key[i];
+                        found = true;
-                log_device_debug(dev, "test_key: checking bit block %lu for any keys; found=%i", (unsigned long)i*BITS_PER_LONG, found > 0);
+
+                log_device_debug(dev, "test_key: checking bit block %zu for any keys; found=%s",
+                                 i * BITS_PER_LONG, yes_no(found));
         }
         /* If there are no keys in the lower block, check the higher blocks */
-        if (!found) {
+        for (size_t block = 0; block < sizeof(high_key_blocks) / sizeof(struct range) && !found; block++)
-                unsigned block;
+                for (unsigned i = high_key_blocks[block].start; i < high_key_blocks[block].end && !found; i++)
-                for (block = 0; block < (sizeof(high_key_blocks) / sizeof(struct range)); ++block) {
-                        for (i = high_key_blocks[block].start; i < high_key_blocks[block].end; ++i) {
                         if (test_bit(i, bitmask_key)) {
                                 log_device_debug(dev, "test_key: Found key %x in high block", i);
-                                        found = 1;
+                                found = true;
-                                        break;
-                                }
-                        }
-                }
                         }
 
-        if (found > 0) {
+        if (found)
                 udev_builtin_add_property(dev, test, "ID_INPUT_KEY", "1");
-                ret = true;
-        }
 
         /* the first 32 bits are ESC, numbers, and Q to D; if we have all of
          * those, consider it a full keyboard; do not test KEY_RESERVED, though */
-        mask = 0xFFFFFFFE;
+        if (FLAGS_SET(bitmask_key[0], 0xFFFFFFFE)) {
-        if (FLAGS_SET(bitmask_key[0], mask)) {
                 udev_builtin_add_property(dev, test, "ID_INPUT_KEYBOARD", "1");
-                ret = true;
+                return true;
         }
 
-        return ret;
+        return found;
 }
 
 static int builtin_input_id(sd_device *dev, int argc, char *argv[], bool test) {

src/udev/udev-builtin-kmod.c

@@ -21,16 +21,14 @@ _printf_(6,0) static void udev_kmod_log(void *data, int priority, const char *fi
 }
 
 static int builtin_kmod(sd_device *dev, int argc, char *argv[], bool test) {
-        int i;
-
         if (!ctx)
                 return 0;
 
         if (argc < 3 || !streq(argv[1], "load"))
                 return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
-                                       "%s: expected: load <module>", argv[0]);
+                                       "%s: expected: load <module>…", argv[0]);
 
-        for (i = 2; argv[i]; i++)
+        for (int i = 2; argv[i]; i++)
                 (void) module_load_and_warn(ctx, argv[i], false);
 
         return 0;

src/udev/udev-builtin-net_id.c

@@ -78,25 +78,23 @@ struct virtfn_info {
 
 /* skip intermediate virtio devices */
 static sd_device *skip_virtio(sd_device *dev) {
-        sd_device *parent;
-
         /* there can only ever be one virtio bus per parent device, so we can
          * safely ignore any virtio buses. see
          * http://lists.linuxfoundation.org/pipermail/virtualization/2015-August/030331.html */
-        for (parent = dev; parent; ) {
+        while (dev) {
                 const char *subsystem;
 
-                if (sd_device_get_subsystem(parent, &subsystem) < 0)
+                if (sd_device_get_subsystem(dev, &subsystem) < 0)
                         break;
 
                 if (!streq(subsystem, "virtio"))
                         break;
 
-                if (sd_device_get_parent(parent, &parent) < 0)
+                if (sd_device_get_parent(dev, &dev) < 0)
                         return NULL;
         }
 
-        return parent;
+        return dev;
 }
 
 static int get_virtfn_info(sd_device *dev, struct netnames *names, struct virtfn_info *ret) {

src/udev/udev-builtin-path_id.c

@@ -80,22 +80,19 @@ static int format_lun_number(sd_device *dev, char **path) {
 }
 
 static sd_device *skip_subsystem(sd_device *dev, const char *subsys) {
-        sd_device *parent;
-
         assert(dev);
         assert(subsys);
 
-        for (parent = dev; ; ) {
+        for (;;) {
                 const char *subsystem;
 
-                if (sd_device_get_subsystem(parent, &subsystem) < 0)
+                if (sd_device_get_subsystem(dev, &subsystem) < 0)
                         break;
 
                 if (!streq(subsystem, subsys))
                         break;
 
-                dev = parent;
+                if (sd_device_get_parent(dev, &dev) < 0)
-                if (sd_device_get_parent(dev, &parent) < 0)
                         break;
         }
 
@@ -378,7 +375,6 @@ static sd_device *handle_scsi_hyperv(sd_device *parent, char **path, size_t guid
         const char *guid_str;
         _cleanup_free_ char *lun = NULL;
         char guid[39];
-        size_t i, k;
 
         assert(parent);
         assert(path);
@@ -396,7 +392,8 @@ static sd_device *handle_scsi_hyperv(sd_device *parent, char **path, size_t guid
         if (strlen(guid_str) < guid_str_len || guid_str[0] != '{' || guid_str[guid_str_len-1] != '}')
                 return NULL;
 
-        for (i = 1, k = 0; i < guid_str_len-1; i++) {
+        size_t k = 0;
+        for (size_t i = 1; i < guid_str_len-1; i++) {
                 if (guid_str[i] == '-')
                         continue;
                 guid[k++] = guid_str[i];
@@ -681,11 +678,10 @@ static int builtin_path_id(sd_device *dev, int argc, char *argv[], bool test) {
 
         {
                 char tag[UDEV_NAME_SIZE];
-                size_t i;
+                size_t i = 0;
-                const char *p;
 
                 /* compose valid udev tag name */
-                for (p = path, i = 0; *p; p++) {
+                for (const char *p = path; *p; p++) {
                         if ((*p >= '0' && *p <= '9') ||
                             (*p >= 'A' && *p <= 'Z') ||
                             (*p >= 'a' && *p <= 'z') ||

src/udev/udev-builtin-usb_id.c

@@ -405,10 +405,8 @@ fallback:
                 const char *usb_serial;
 
                 if (sd_device_get_sysattr_value(dev_usb, "serial", &usb_serial) >= 0) {
-                        const unsigned char *p;
-
                         /* http://msdn.microsoft.com/en-us/library/windows/hardware/gg487321.aspx */
-                        for (p = (unsigned char *) usb_serial; *p != '\0'; p++)
+                        for (const unsigned char *p = (unsigned char*) usb_serial; *p != '\0'; p++)
                                 if (*p < 0x20 || *p > 0x7f || *p == ',') {
                                         usb_serial = NULL;
                                         break;