[systemd-devel] [PATCH rebased] Use strlen even for constant strings

Josh Triplett josh at joshtriplett.org
Sat Mar 15 11:40:07 PDT 2014


GCC optimizes strlen("string constant") to a constant, even with -O0.
Thus, replace patterns like sizeof("string constant")-1 with
strlen("string constant") where possible, for clarity.  In particular,
for expressions intended to add up the lengths of components going into
a string, this often makes it clearer that the expression counts the
trailing '\0' exactly once, by putting the +1 for the '\0' at the end of
the expression, rather than hidden in a sizeof in the middle of the
expression.
---

On Sat, Mar 15, 2014 at 05:37:49AM +0100, Zbigniew Jędrzejewski-Szmek wrote:
> strlen() is indeed much nicer. Unfortunately it stopped applying cleanly...
> Could you rebase this?

Done; git rebased it automatically with no conflicts.  (am -3 might work
in the future for patches that trivially rebase.)

 src/core/dbus.c                    |  2 +-
 src/core/service.c                 |  2 +-
 src/journal/journald-server.c      |  2 +-
 src/journal/journald-syslog.c      |  2 +-
 src/libsystemd/sd-bus/bus-kernel.c |  4 ++--
 src/libsystemd/sd-bus/bus-socket.c |  4 ++--
 src/libsystemd/sd-bus/bus-track.c  |  2 +-
 src/shared/cgroup-util.c           |  2 +-
 src/shared/unit-name.c             |  2 +-
 src/shared/util.c                  |  2 +-
 src/sysctl/sysctl.c                |  2 +-
 src/systemctl/systemctl.c          | 40 +++++++++++++++++++-------------------
 src/udev/udev-rules.c              | 14 ++++++-------
 13 files changed, 40 insertions(+), 40 deletions(-)

diff --git a/src/core/dbus.c b/src/core/dbus.c
index 03f3738..72f36bd 100644
--- a/src/core/dbus.c
+++ b/src/core/dbus.c
@@ -953,7 +953,7 @@ static int bus_init_private(Manager *m) {
                         return 0;
 
                 strcpy(sa.un.sun_path, "/run/systemd/private");
-                salen = offsetof(union sockaddr_union, un.sun_path) + sizeof("/run/systemd/private") - 1;
+                salen = offsetof(union sockaddr_union, un.sun_path) + strlen("/run/systemd/private");
         } else {
                 size_t left = sizeof(sa.un.sun_path);
                 char *p = sa.un.sun_path;
diff --git a/src/core/service.c b/src/core/service.c
index 121ddec..41b95ab 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -361,7 +361,7 @@ static int service_arm_timer(Service *s, usec_t usec) {
 static char *sysv_translate_name(const char *name) {
         char *r;
 
-        r = new(char, strlen(name) + sizeof(".service"));
+        r = new(char, strlen(name) + strlen(".service") + 1);
         if (!r)
                 return NULL;
 
diff --git a/src/journal/journald-server.c b/src/journal/journald-server.c
index f0117e7..5befe93 100644
--- a/src/journal/journald-server.c
+++ b/src/journal/journald-server.c
@@ -686,7 +686,7 @@ static void dispatch_message_real(
 #ifdef HAVE_SELINUX
                 if (use_selinux()) {
                         if (label) {
-                                x = alloca(sizeof("_SELINUX_CONTEXT=") + label_len);
+                                x = alloca(strlen("_SELINUX_CONTEXT=") + label_len + 1);
 
                                 *((char*) mempcpy(stpcpy(x, "_SELINUX_CONTEXT="), label, label_len)) = 0;
                                 IOVEC_SET_STRING(iovec[n++], x);
diff --git a/src/journal/journald-syslog.c b/src/journal/journald-syslog.c
index cbb944f..fee7d91 100644
--- a/src/journal/journald-syslog.c
+++ b/src/journal/journald-syslog.c
@@ -46,7 +46,7 @@ static void forward_syslog_iovec(Server *s, const struct iovec *iovec, unsigned
                 .msg_iovlen = n_iovec,
                 .msg_name = &sa,
                 .msg_namelen = offsetof(union sockaddr_union, un.sun_path)
-                               + sizeof("/run/systemd/journal/syslog") - 1,
+                               + strlen("/run/systemd/journal/syslog"),
         };
         struct cmsghdr *cmsg;
         union {
diff --git a/src/libsystemd/sd-bus/bus-kernel.c b/src/libsystemd/sd-bus/bus-kernel.c
index 80ef15b..5c955f4 100644
--- a/src/libsystemd/sd-bus/bus-kernel.c
+++ b/src/libsystemd/sd-bus/bus-kernel.c
@@ -1390,7 +1390,7 @@ int bus_kernel_create_starter(const char *bus, const char *name, BusNamePolicy *
         assert(bus);
         assert(name);
 
-        p = alloca(sizeof("/dev/kdbus/") - 1 + DECIMAL_STR_MAX(uid_t) + 1 + strlen(bus) + sizeof("/bus"));
+        p = alloca(strlen("/dev/kdbus/") + DECIMAL_STR_MAX(uid_t) + 1 + strlen(bus) + strlen("/bus") + 1);
         sprintf(p, "/dev/kdbus/%lu-%s/bus", (unsigned long) getuid(), bus);
 
         fd = open(p, O_RDWR|O_NOCTTY|O_CLOEXEC);
@@ -1502,7 +1502,7 @@ int bus_kernel_create_monitor(const char *bus) {
 
         assert(bus);
 
-        p = alloca(sizeof("/dev/kdbus/") - 1 + DECIMAL_STR_MAX(uid_t) + 1 + strlen(bus) + sizeof("/bus"));
+        p = alloca(strlen("/dev/kdbus/") + DECIMAL_STR_MAX(uid_t) + 1 + strlen(bus) + strlen("/bus") + 1);
         sprintf(p, "/dev/kdbus/%lu-%s/bus", (unsigned long) getuid(), bus);
 
         fd = open(p, O_RDWR|O_NOCTTY|O_CLOEXEC);
diff --git a/src/libsystemd/sd-bus/bus-socket.c b/src/libsystemd/sd-bus/bus-socket.c
index 0c4b6af..016f8a1 100644
--- a/src/libsystemd/sd-bus/bus-socket.c
+++ b/src/libsystemd/sd-bus/bus-socket.c
@@ -227,8 +227,8 @@ static int bus_socket_auth_verify_client(sd_bus *b) {
 
         if (f)
                 b->can_fds =
-                        (f - e == sizeof("\r\nAGREE_UNIX_FD") - 1) &&
-                        memcmp(e + 2, "AGREE_UNIX_FD", sizeof("AGREE_UNIX_FD") - 1) == 0;
+                        (f - e == strlen("\r\nAGREE_UNIX_FD")) &&
+                        memcmp(e + 2, "AGREE_UNIX_FD", strlen("AGREE_UNIX_FD")) == 0;
 
         b->rbuffer_size -= (start - (char*) b->rbuffer);
         memmove(b->rbuffer, start, b->rbuffer_size);
diff --git a/src/libsystemd/sd-bus/bus-track.c b/src/libsystemd/sd-bus/bus-track.c
index fd368e4..e21959d 100644
--- a/src/libsystemd/sd-bus/bus-track.c
+++ b/src/libsystemd/sd-bus/bus-track.c
@@ -51,7 +51,7 @@ struct sd_bus_track {
         ({                                                              \
                 char *_x;                                               \
                 size_t _l = strlen(name);                               \
-                _x = alloca(sizeof(MATCH_PREFIX)-1+_l+sizeof(MATCH_SUFFIX)); \
+                _x = alloca(strlen(MATCH_PREFIX)+_l+strlen(MATCH_SUFFIX)+1); \
                 strcpy(stpcpy(stpcpy(_x, MATCH_PREFIX), name), MATCH_SUFFIX); \
                 _x;                                                     \
         })
diff --git a/src/shared/cgroup-util.c b/src/shared/cgroup-util.c
index 06eb453..9d50890 100644
--- a/src/shared/cgroup-util.c
+++ b/src/shared/cgroup-util.c
@@ -509,7 +509,7 @@ static int check_hierarchy(const char *p) {
         assert(p);
 
         /* Check if this controller actually really exists */
-        cc = alloca(sizeof("/sys/fs/cgroup/") + strlen(p));
+        cc = alloca(strlen("/sys/fs/cgroup/") + strlen(p) + 1);
         strcpy(stpcpy(cc, "/sys/fs/cgroup/"), p);
         if (access(cc, F_OK) < 0)
                 return -errno;
diff --git a/src/shared/unit-name.c b/src/shared/unit-name.c
index e9b0636..6c167b4 100644
--- a/src/shared/unit-name.c
+++ b/src/shared/unit-name.c
@@ -493,7 +493,7 @@ char *unit_name_mangle(const char *name, enum unit_name_mangle allow_globs) {
         /* We'll only escape the obvious characters here, to play
          * safe. */
 
-        r = new(char, strlen(name) * 4 + 1 + sizeof(".service")-1);
+        r = new(char, strlen(name) * 4 + strlen(".service") + 1);
         if (!r)
                 return NULL;
 
diff --git a/src/shared/util.c b/src/shared/util.c
index 7e17851..75870fc 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -4185,7 +4185,7 @@ int socket_from_display(const char *display, char **path) {
 
         k = strspn(display+1, "0123456789");
 
-        f = new(char, sizeof("/tmp/.X11-unix/X") + k);
+        f = new(char, strlen("/tmp/.X11-unix/X") + k + 1);
         if (!f)
                 return -ENOMEM;
 
diff --git a/src/sysctl/sysctl.c b/src/sysctl/sysctl.c
index 8868732..283eefe 100644
--- a/src/sysctl/sysctl.c
+++ b/src/sysctl/sysctl.c
@@ -65,7 +65,7 @@ static int apply_sysctl(const char *property, const char *value) {
 
         log_debug("Setting '%s' to '%s'", property, value);
 
-        p = new(char, sizeof("/proc/sys/") + strlen(property));
+        p = new(char, strlen("/proc/sys/") + strlen(property) + 1);
         if (!p)
                 return log_oom();
 
diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index 156605d..fc872bd 100644
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -333,11 +333,11 @@ static void output_units_list(const UnitInfo *unit_infos, unsigned c) {
         unsigned n_shown = 0;
         int job_count = 0;
 
-        max_id_len = sizeof("UNIT")-1;
-        load_len = sizeof("LOAD")-1;
-        active_len = sizeof("ACTIVE")-1;
-        sub_len = sizeof("SUB")-1;
-        job_len = sizeof("JOB")-1;
+        max_id_len = strlen("UNIT");
+        load_len = strlen("LOAD");
+        active_len = strlen("ACTIVE");
+        sub_len = strlen("SUB");
+        job_len = strlen("JOB");
         desc_len = 0;
 
         for (u = unit_infos; u < unit_infos + c; u++) {
@@ -639,10 +639,10 @@ static int socket_info_compare(const struct socket_info *a, const struct socket_
 
 static int output_sockets_list(struct socket_info *socket_infos, unsigned cs) {
         struct socket_info *s;
-        unsigned pathlen = sizeof("LISTEN") - 1,
-                typelen = (sizeof("TYPE") - 1) * arg_show_types,
-                socklen = sizeof("UNIT") - 1,
-                servlen = sizeof("ACTIVATES") - 1;
+        unsigned pathlen = strlen("LISTEN"),
+                typelen = strlen("TYPE") * arg_show_types,
+                socklen = strlen("UNIT"),
+                servlen = strlen("ACTIVATES");
         const char *on, *off;
 
         for (s = socket_infos; s < socket_infos + cs; s++) {
@@ -836,10 +836,10 @@ static int timer_info_compare(const struct timer_info *a, const struct timer_inf
 static int output_timers_list(struct timer_info *timer_infos, unsigned n) {
         struct timer_info *t;
         unsigned
-                nextlen = sizeof("NEXT") - 1,
-                leftlen = sizeof("LEFT") - 1,
-                unitlen = sizeof("UNIT") - 1,
-                activatelen = sizeof("ACTIVATES") - 1;
+                nextlen = strlen("NEXT"),
+                leftlen = strlen("LEFT"),
+                unitlen = strlen("UNIT"),
+                activatelen = strlen("ACTIVATES");
 
         const char *on, *off;
 
@@ -1034,8 +1034,8 @@ static void output_unit_file_list(const UnitFileList *units, unsigned c) {
         unsigned max_id_len, id_cols, state_cols;
         const UnitFileList *u;
 
-        max_id_len = sizeof("UNIT FILE")-1;
-        state_cols = sizeof("STATE")-1;
+        max_id_len = strlen("UNIT FILE");
+        state_cols = strlen("STATE");
 
         for (u = units; u < units + c; u++) {
                 max_id_len = MAX(max_id_len, strlen(basename(u->path)));
@@ -1792,10 +1792,10 @@ static void output_jobs_list(const struct job_info* jobs, unsigned n, bool skipp
 
         pager_open_if_enabled();
 
-        id_len = sizeof("JOB")-1;
-        unit_len = sizeof("UNIT")-1;
-        type_len = sizeof("TYPE")-1;
-        state_len = sizeof("STATE")-1;
+        id_len = strlen("JOB");
+        unit_len = strlen("UNIT");
+        type_len = strlen("TYPE");
+        state_len = strlen("STATE");
 
         for (j = jobs; j < jobs + n; j++) {
                 uint32_t id = j->id;
@@ -6332,7 +6332,7 @@ static int send_shutdownd(usec_t t, char mode, bool dry_run, bool warn, const ch
         struct msghdr msghdr = {
                 .msg_name = &sockaddr,
                 .msg_namelen = offsetof(struct sockaddr_un, sun_path)
-                               + sizeof("/run/systemd/shutdownd") - 1,
+                               + strlen("/run/systemd/shutdownd"),
                 .msg_iov = iovec,
                 .msg_iovlen = 1,
         };
diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c
index 47bde61..2630264 100644
--- a/src/udev/udev-rules.c
+++ b/src/udev/udev-rules.c
@@ -1142,7 +1142,7 @@ static int add_rule(struct udev_rules *rules, char *line,
                 }
 
                 if (startswith(key, "ATTR{")) {
-                        attr = get_key_attribute(rules->udev, key + sizeof("ATTR")-1);
+                        attr = get_key_attribute(rules->udev, key + strlen("ATTR"));
                         if (attr == NULL) {
                                 log_error("error parsing ATTR attribute");
                                 goto invalid;
@@ -1156,7 +1156,7 @@ static int add_rule(struct udev_rules *rules, char *line,
                 }
 
                 if (startswith(key, "SECLABEL{")) {
-                        attr = get_key_attribute(rules->udev, key + sizeof("SECLABEL")-1);
+                        attr = get_key_attribute(rules->udev, key + strlen("SECLABEL"));
                         if (!attr) {
                                 log_error("error parsing SECLABEL attribute");
                                 goto invalid;
@@ -1198,7 +1198,7 @@ static int add_rule(struct udev_rules *rules, char *line,
                                 log_error("invalid ATTRS operation");
                                 goto invalid;
                         }
-                        attr = get_key_attribute(rules->udev, key + sizeof("ATTRS")-1);
+                        attr = get_key_attribute(rules->udev, key + strlen("ATTRS"));
                         if (attr == NULL) {
                                 log_error("error parsing ATTRS attribute");
                                 goto invalid;
@@ -1223,7 +1223,7 @@ static int add_rule(struct udev_rules *rules, char *line,
                 }
 
                 if (startswith(key, "ENV{")) {
-                        attr = get_key_attribute(rules->udev, key + sizeof("ENV")-1);
+                        attr = get_key_attribute(rules->udev, key + strlen("ENV"));
                         if (attr == NULL) {
                                 log_error("error parsing ENV attribute");
                                 goto invalid;
@@ -1282,7 +1282,7 @@ static int add_rule(struct udev_rules *rules, char *line,
                 }
 
                 if (startswith(key, "IMPORT")) {
-                        attr = get_key_attribute(rules->udev, key + sizeof("IMPORT")-1);
+                        attr = get_key_attribute(rules->udev, key + strlen("IMPORT"));
                         if (attr == NULL) {
                                 log_error("IMPORT{} type missing, ignoring IMPORT %s:%u", filename, lineno);
                                 continue;
@@ -1328,7 +1328,7 @@ static int add_rule(struct udev_rules *rules, char *line,
                                 log_error("invalid TEST operation");
                                 goto invalid;
                         }
-                        attr = get_key_attribute(rules->udev, key + sizeof("TEST")-1);
+                        attr = get_key_attribute(rules->udev, key + strlen("TEST"));
                         if (attr != NULL) {
                                 mode = strtol(attr, NULL, 8);
                                 rule_add_key(&rule_tmp, TK_M_TEST, op, value, &mode);
@@ -1339,7 +1339,7 @@ static int add_rule(struct udev_rules *rules, char *line,
                 }
 
                 if (startswith(key, "RUN")) {
-                        attr = get_key_attribute(rules->udev, key + sizeof("RUN")-1);
+                        attr = get_key_attribute(rules->udev, key + strlen("RUN"));
                         if (attr == NULL)
                                 attr = "program";
 
-- 
1.9.0



More information about the systemd-devel mailing list