[systemd-devel] [PATCH] Use strlen even for constant strings
Josh Triplett
josh at joshtriplett.org
Sat Mar 8 22:54:36 PST 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.
---
Motivation: staring at a pile of string handling code, carefully vetting
it for off-by-one errors, and encountering one too many size expressions
whose handling of the trailing '\0' didn't instantly follow from a
glance at the expression.
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 8680650..7595248 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 241f7ed..eadc0c4 100644
--- a/src/journal/journald-syslog.c
+++ b/src/journal/journald-syslog.c
@@ -45,7 +45,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 2a1b0b4..d339090 100644
--- a/src/libsystemd/sd-bus/bus-kernel.c
+++ b/src/libsystemd/sd-bus/bus-kernel.c
@@ -1389,7 +1389,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);
@@ -1501,7 +1501,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 d28caae..c5dad11 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -4190,7 +4190,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 76efacb..de6ce7c 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 d7983d1..acdad50 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;
@@ -1032,8 +1032,8 @@ static void output_unit_file_list(const UnitFileList *units, unsigned c) {
unsigned max_id_len, id_cols, state_cols, n_shown = 0;
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)));
@@ -1570,10 +1570,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;
@@ -6024,7 +6024,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