From fbbe6d65b49454c84fb538dcd7cce6099d678007 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 11 Jun 2018 13:47:25 +0200 Subject: [PATCH 1/5] basic/parse-util: remove unnecessary parentheses --- src/basic/parse-util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/basic/parse-util.c b/src/basic/parse-util.c index b25bc6e2151..33698b06dac 100644 --- a/src/basic/parse-util.c +++ b/src/basic/parse-util.c @@ -662,7 +662,7 @@ int parse_permille_unbounded(const char *p) { r = safe_atoi(n, &v); if (r < 0) return r; - if (v > ((INT_MAX - q) / 10)) + if (v > (INT_MAX - q) / 10) return -ERANGE; v = v * 10 + q; From 3b253ad6890af0fda91fed8b67ad0805b6ff1a3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 11 Jun 2018 15:58:09 +0200 Subject: [PATCH 2/5] cocinelle: use GNU parallel to run spatch spatch is single-threaded, i.e. slow. On my machine it allocates 5 GB of memory and starts swapping, which makes it even slower. Using parallel makes the whole thing pleasantly fast. --- coccinelle/run-coccinelle.sh | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/coccinelle/run-coccinelle.sh b/coccinelle/run-coccinelle.sh index a18da58a3ec..22ab66d3dd5 100755 --- a/coccinelle/run-coccinelle.sh +++ b/coccinelle/run-coccinelle.sh @@ -11,11 +11,17 @@ case "$1" in ;; esac +if ! parallel -h >/dev/null; then + echo 'Please install GNU parallel (package "parallel")' + exit 1 +fi + for SCRIPT in ${@-$top/coccinelle/*.cocci} ; do echo "--x-- Processing $SCRIPT --x--" TMPFILE=`mktemp` echo "+ spatch --sp-file $SCRIPT $args ..." - spatch --sp-file $SCRIPT $args $files 2>"$TMPFILE" || cat "$TMPFILE" - rm "$TMPFILE" + parallel --halt now,fail=1 --keep-order --noswap --max-args=20 \ + spatch --sp-file $SCRIPT $args ::: $files \ + 2>"$TMPFILE" || cat "$TMPFILE" echo -e "--x-- Processed $SCRIPT --x--\n" done From 37e744e8661c3edd8b8efa723a3d3c71f73ef542 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 11 Jun 2018 16:07:45 +0200 Subject: [PATCH 3/5] test-alloc-util: add a "test" for bool casts Just in case ;) There is no good place, test-alloc-util.c is as good as any, and it's quite short so far, so let's add this there. --- src/test/test-alloc-util.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/test/test-alloc-util.c b/src/test/test-alloc-util.c index 578c4e2d272..3343e6eecf4 100644 --- a/src/test/test-alloc-util.c +++ b/src/test/test-alloc-util.c @@ -57,9 +57,26 @@ static void test_memdup_multiply_and_greedy_realloc(void) { assert_se(p[i] == 0); } +static void test_bool_assign(void) { + bool b, c, *cp = &c, d, e, f; + + b = 123; + *cp = -11; + d = 0xF & 0xFF; + e = b & d; + f = 0x0; + + assert(b); + assert(c); + assert(d); + assert(e); + assert(!f); +} + int main(int argc, char *argv[]) { test_alloca(); test_memdup_multiply_and_greedy_realloc(); + test_bool_assign(); return 0; } From 5d904a6aaaceae7fe2f11d6b848b0dd45e3fd1c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 11 Jun 2018 16:02:03 +0200 Subject: [PATCH 4/5] tree-wide: drop !! casts to booleans They are not needed, because anything that is non-zero is converted to true. C11: > 6.3.1.2: When any scalar value is converted to _Bool, the result is 0 if the > value compares equal to 0; otherwise, the result is 1. https://stackoverflow.com/questions/31551888/casting-int-to-bool-in-c-c --- coccinelle/bool-cast.cocci | 12 ++++++++++++ src/analyze/analyze.c | 4 ++-- src/basic/btrfs-util.c | 2 +- src/basic/format-table.c | 2 +- src/basic/verbs.c | 2 +- src/basic/virt.c | 2 +- src/busctl/busctl.c | 10 +++++----- src/core/dbus-execute.c | 2 +- src/core/namespace.c | 2 +- src/core/service.c | 2 +- src/core/shutdown.c | 2 +- src/journal/journald-stream.c | 8 ++++---- src/login/logind-dbus.c | 4 ++-- src/network/networkd-dhcp6.c | 2 +- src/shared/bus-util.c | 6 +++--- src/shared/condition.c | 2 +- src/shared/conf-parser.c | 2 +- src/timedate/timedatectl.c | 2 +- 18 files changed, 40 insertions(+), 28 deletions(-) create mode 100644 coccinelle/bool-cast.cocci diff --git a/coccinelle/bool-cast.cocci b/coccinelle/bool-cast.cocci new file mode 100644 index 00000000000..051ccb94175 --- /dev/null +++ b/coccinelle/bool-cast.cocci @@ -0,0 +1,12 @@ +@@ +bool b; +expression y; +@@ +- b = !!(y); ++ b = y; +@@ +bool b; +expression y; +@@ +- b = !!y; ++ b = y; diff --git a/src/analyze/analyze.c b/src/analyze/analyze.c index 8695e7a1d2a..906ddfbc280 100644 --- a/src/analyze/analyze.c +++ b/src/analyze/analyze.c @@ -1833,7 +1833,7 @@ static int parse_argv(int argc, char *argv[]) { return -EINVAL; } - arg_man = !!r; + arg_man = r; } else arg_man = true; @@ -1847,7 +1847,7 @@ static int parse_argv(int argc, char *argv[]) { return -EINVAL; } - arg_generators = !!r; + arg_generators = r; } else arg_generators = true; diff --git a/src/basic/btrfs-util.c b/src/basic/btrfs-util.c index 32a022f7ce1..b325090ed9c 100644 --- a/src/basic/btrfs-util.c +++ b/src/basic/btrfs-util.c @@ -500,7 +500,7 @@ int btrfs_subvol_get_info_fd(int fd, uint64_t subvol_id, BtrfsSubvolInfo *ret) { (usec_t) le32toh(ri->otime.nsec) / NSEC_PER_USEC; ret->subvol_id = subvol_id; - ret->read_only = !!(le64toh(ri->flags) & BTRFS_ROOT_SUBVOL_RDONLY); + ret->read_only = le64toh(ri->flags) & BTRFS_ROOT_SUBVOL_RDONLY; assert_cc(sizeof(ri->uuid) == sizeof(ret->uuid)); memcpy(&ret->uuid, ri->uuid, sizeof(ret->uuid)); diff --git a/src/basic/format-table.c b/src/basic/format-table.c index ad89556ee2c..94e796d1caa 100644 --- a/src/basic/format-table.c +++ b/src/basic/format-table.c @@ -572,7 +572,7 @@ int table_add_many_internal(Table *t, TableDataType first_type, ...) { break; case TABLE_BOOLEAN: - buffer.b = !!va_arg(ap, int); + buffer.b = va_arg(ap, int); data = &buffer.b; break; diff --git a/src/basic/verbs.c b/src/basic/verbs.c index 58076ac07c9..057e35dcd0b 100644 --- a/src/basic/verbs.c +++ b/src/basic/verbs.c @@ -81,7 +81,7 @@ int dispatch_verb(int argc, char *argv[], const Verb verbs[], void *userdata) { if (name) found = streq(name, verbs[i].verb); else - found = !!(verbs[i].flags & VERB_DEFAULT); + found = verbs[i].flags & VERB_DEFAULT; if (found) { verb = &verbs[i]; diff --git a/src/basic/virt.c b/src/basic/virt.c index 9da5feca6cc..0ebccd4ebb3 100644 --- a/src/basic/virt.c +++ b/src/basic/virt.c @@ -56,7 +56,7 @@ static int detect_vm_cpuid(void) { if (__get_cpuid(1, &eax, &ebx, &ecx, &edx) == 0) return VIRTUALIZATION_NONE; - hypervisor = !!(ecx & 0x80000000U); + hypervisor = ecx & 0x80000000U; if (hypervisor) { union { diff --git a/src/busctl/busctl.c b/src/busctl/busctl.c index 5c65147caca..d6844057d38 100644 --- a/src/busctl/busctl.c +++ b/src/busctl/busctl.c @@ -1972,7 +1972,7 @@ static int parse_argv(int argc, char *argv[]) { return r; } - arg_expect_reply = !!r; + arg_expect_reply = r; break; case ARG_AUTO_START: @@ -1982,7 +1982,7 @@ static int parse_argv(int argc, char *argv[]) { return r; } - arg_auto_start = !!r; + arg_auto_start = r; break; case ARG_ALLOW_INTERACTIVE_AUTHORIZATION: @@ -1992,7 +1992,7 @@ static int parse_argv(int argc, char *argv[]) { return r; } - arg_allow_interactive_authorization = !!r; + arg_allow_interactive_authorization = r; break; case ARG_TIMEOUT: @@ -2011,7 +2011,7 @@ static int parse_argv(int argc, char *argv[]) { return r; } - arg_augment_creds = !!r; + arg_augment_creds = r; break; case ARG_WATCH_BIND: @@ -2021,7 +2021,7 @@ static int parse_argv(int argc, char *argv[]) { return r; } - arg_watch_bind = !!r; + arg_watch_bind = r; break; case '?': diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c index 747b9d8eeb1..52d0487c93c 100644 --- a/src/core/dbus-execute.c +++ b/src/core/dbus-execute.c @@ -563,7 +563,7 @@ static int property_get_bind_paths( assert(property); assert(reply); - ro = !!strstr(property, "ReadOnly"); + ro = strstr(property, "ReadOnly"); r = sd_bus_message_open_container(reply, 'a', "(ssbt)"); if (r < 0) diff --git a/src/core/namespace.c b/src/core/namespace.c index 2523c2a47f4..bb19e18e435 100644 --- a/src/core/namespace.c +++ b/src/core/namespace.c @@ -322,7 +322,7 @@ static int append_tmpfs_mounts(MountEntry **p, const TemporaryFileSystem *tmpfs, if (r < 0) return r; - ro = !!(flags & MS_RDONLY); + ro = flags & MS_RDONLY; if (ro) flags ^= MS_RDONLY; } diff --git a/src/core/service.c b/src/core/service.c index 33fd36f8a7c..824d283f343 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -3215,7 +3215,7 @@ static void service_sigchld_event(Unit *u, pid_t pid, int code, int status) { * The PID file might actually be created by a START_POST * script. In that case don't worry if the loading fails. */ - has_start_post = !!s->exec_command[SERVICE_EXEC_START_POST]; + has_start_post = s->exec_command[SERVICE_EXEC_START_POST]; r = service_load_pid_file(s, !has_start_post); if (!has_start_post && r < 0) { r = service_demand_pid_file(s); diff --git a/src/core/shutdown.c b/src/core/shutdown.c index 63fd4c8da18..ffd80d7876b 100644 --- a/src/core/shutdown.c +++ b/src/core/shutdown.c @@ -302,7 +302,7 @@ int main(int argc, char *argv[]) { (void) cg_get_root_path(&cgroup); in_container = detect_container() > 0; - use_watchdog = !!getenv("WATCHDOG_USEC"); + use_watchdog = getenv("WATCHDOG_USEC"); watchdog_device = getenv("WATCHDOG_DEVICE"); if (watchdog_device) { r = watchdog_set_device(watchdog_device); diff --git a/src/journal/journald-stream.c b/src/journal/journald-stream.c index 4975aba016a..5cf9f40c202 100644 --- a/src/journal/journald-stream.c +++ b/src/journal/journald-stream.c @@ -378,7 +378,7 @@ static int stdout_stream_line(StdoutStream *s, char *p, LineBreak line_break) { return -EINVAL; } - s->level_prefix = !!r; + s->level_prefix = r; s->state = STDOUT_STREAM_FORWARD_TO_SYSLOG; return 0; @@ -389,7 +389,7 @@ static int stdout_stream_line(StdoutStream *s, char *p, LineBreak line_break) { return -EINVAL; } - s->forward_to_syslog = !!r; + s->forward_to_syslog = r; s->state = STDOUT_STREAM_FORWARD_TO_KMSG; return 0; @@ -400,7 +400,7 @@ static int stdout_stream_line(StdoutStream *s, char *p, LineBreak line_break) { return -EINVAL; } - s->forward_to_kmsg = !!r; + s->forward_to_kmsg = r; s->state = STDOUT_STREAM_FORWARD_TO_CONSOLE; return 0; @@ -411,7 +411,7 @@ static int stdout_stream_line(StdoutStream *s, char *p, LineBreak line_break) { return -EINVAL; } - s->forward_to_console = !!r; + s->forward_to_console = r; s->state = STDOUT_STREAM_RUNNING; /* Try to save the stream, so that journald can be restarted and we can recover */ diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c index 3fe38c0527f..21fdc0912dc 100644 --- a/src/login/logind-dbus.c +++ b/src/login/logind-dbus.c @@ -242,9 +242,9 @@ static int property_get_preparing( assert(m); if (streq(property, "PreparingForShutdown")) - b = !!(m->action_what & INHIBIT_SHUTDOWN); + b = m->action_what & INHIBIT_SHUTDOWN; else - b = !!(m->action_what & INHIBIT_SLEEP); + b = m->action_what & INHIBIT_SLEEP; return sd_bus_message_append(reply, "b", b); } diff --git a/src/network/networkd-dhcp6.c b/src/network/networkd-dhcp6.c index e9db977036d..3f8346fee8a 100644 --- a/src/network/networkd-dhcp6.c +++ b/src/network/networkd-dhcp6.c @@ -406,7 +406,7 @@ int dhcp6_request_address(Link *link, int ir) { if (r < 0) return r; else - running = !!r; + running = r; if (running) { r = sd_dhcp6_client_get_information_request(link->dhcp6_client, &inf_req); diff --git a/src/shared/bus-util.c b/src/shared/bus-util.c index bae419d63bc..ddbc9d934ef 100644 --- a/src/shared/bus-util.c +++ b/src/shared/bus-util.c @@ -1090,9 +1090,9 @@ static int map_basic(sd_bus *bus, const char *member, sd_bus_message *m, unsigne return r; if (flags & BUS_MAP_BOOLEAN_AS_BOOL) - * (bool*) userdata = !!b; + *(bool*) userdata = b; else - * (int*) userdata = b; + *(int*) userdata = b; return 0; } @@ -1403,7 +1403,7 @@ int bus_property_set_bool( if (r < 0) return r; - *(bool *) userdata = !!b; + *(bool*) userdata = b; return 0; } diff --git a/src/shared/condition.c b/src/shared/condition.c index a41a7826644..18ab1f89f95 100644 --- a/src/shared/condition.c +++ b/src/shared/condition.c @@ -103,7 +103,7 @@ static int condition_test_kernel_command_line(Condition *c) { if (r < 0) return r; - equal = !!strchr(c->parameter, '='); + equal = strchr(c->parameter, '='); for (p = line;;) { _cleanup_free_ char *word = NULL; diff --git a/src/shared/conf-parser.c b/src/shared/conf-parser.c index 4a41b814aea..b1250850d5a 100644 --- a/src/shared/conf-parser.c +++ b/src/shared/conf-parser.c @@ -632,7 +632,7 @@ int config_parse_bool(const char* unit, return fatal ? -ENOEXEC : 0; } - *b = !!k; + *b = k; return 0; } diff --git a/src/timedate/timedatectl.c b/src/timedate/timedatectl.c index e52a64a9c50..f4120822656 100644 --- a/src/timedate/timedatectl.c +++ b/src/timedate/timedatectl.c @@ -477,7 +477,7 @@ static int map_ntp_message(sd_bus *bus, const char *member, sd_bus_message *m, s memcpy(p->reference.str, d, sz); - p->spike = !!b; + p->spike = b; return 0; } From 108ccae9e193971982d242929d7dc069d780a01e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 13 Jun 2018 10:34:30 +0200 Subject: [PATCH 5/5] test-alloc-util: add casts to bools from p ointers C++03: "An rvalue of arithmetic, enumeration, pointer, or pointer to member type can be converted to an rvalue of type bool. A zero value, null pointer value, or null member pointer value is converted to false; any other value is converted to true" C should behave the same because pointers are scalars in C, but let's verify that. --- src/test/test-alloc-util.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/test/test-alloc-util.c b/src/test/test-alloc-util.c index 3343e6eecf4..549925126e0 100644 --- a/src/test/test-alloc-util.c +++ b/src/test/test-alloc-util.c @@ -58,19 +58,23 @@ static void test_memdup_multiply_and_greedy_realloc(void) { } static void test_bool_assign(void) { - bool b, c, *cp = &c, d, e, f; + bool b, c, *cp = &c, d, e, f, g, h; b = 123; *cp = -11; d = 0xF & 0xFF; e = b & d; f = 0x0; + g = cp; /* cast from pointer */ + h = NULL; /* cast from pointer */ assert(b); assert(c); assert(d); assert(e); assert(!f); + assert(g); + assert(!h); } int main(int argc, char *argv[]) {