From ac097c841e05bcc96da7f369682b56164671eec8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 1 Nov 2017 22:32:22 +0100 Subject: [PATCH 1/7] Remove a bunch of unused variables gcc does not warn about those, because of the _cleanup_ usage. clang is smarter here. --- src/basic/terminal-util.c | 1 - src/core/dbus-execute.c | 2 -- src/gpt-auto-generator/gpt-auto-generator.c | 2 +- src/libsystemd/sd-bus/test-bus-benchmark.c | 2 +- src/login/logind-session.c | 1 - src/network/networkd-routing-policy-rule.c | 3 +-- src/shared/path-lookup.c | 1 - src/test/test-fileio.c | 1 - 8 files changed, 3 insertions(+), 10 deletions(-) diff --git a/src/basic/terminal-util.c b/src/basic/terminal-util.c index 28c8c35fe0..fdf2b098f6 100644 --- a/src/basic/terminal-util.c +++ b/src/basic/terminal-util.c @@ -245,7 +245,6 @@ int ask_string(char **ret, const char *text, ...) { int reset_terminal_fd(int fd, bool switch_to_text) { struct termios termios; - _cleanup_free_ char *utf8 = NULL; int r = 0; /* Set terminal to some sane defaults */ diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c index 26c6e039b8..4dbd6b0f6f 100644 --- a/src/core/dbus-execute.c +++ b/src/core/dbus-execute.c @@ -1177,8 +1177,6 @@ int bus_exec_context_set_transient_property( return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid personality"); if (mode != UNIT_CHECK) { - _cleanup_free_ char *str = NULL; - c->personality = p; unit_write_drop_in_private_format(u, mode, name, "%s=%s", name, s); } diff --git a/src/gpt-auto-generator/gpt-auto-generator.c b/src/gpt-auto-generator/gpt-auto-generator.c index ae0a8da63a..f9a0a14d9e 100644 --- a/src/gpt-auto-generator/gpt-auto-generator.c +++ b/src/gpt-auto-generator/gpt-auto-generator.c @@ -56,7 +56,7 @@ static bool arg_root_enabled = true; static bool arg_root_rw = false; static int add_cryptsetup(const char *id, const char *what, bool rw, bool require, char **device) { - _cleanup_free_ char *e = NULL, *n = NULL, *p = NULL, *d = NULL, *to = NULL; + _cleanup_free_ char *e = NULL, *n = NULL, *p = NULL, *d = NULL; _cleanup_fclose_ FILE *f = NULL; char *ret; int r; diff --git a/src/libsystemd/sd-bus/test-bus-benchmark.c b/src/libsystemd/sd-bus/test-bus-benchmark.c index 6f0ca47df6..2bb435b976 100644 --- a/src/libsystemd/sd-bus/test-bus-benchmark.c +++ b/src/libsystemd/sd-bus/test-bus-benchmark.c @@ -228,7 +228,7 @@ int main(int argc, char *argv[]) { } mode = MODE_BISECT; Type type = TYPE_LEGACY; int i, pair[2] = { -1, -1 }; - _cleanup_free_ char *name = NULL, *bus_name = NULL, *address = NULL, *server_name = NULL; + _cleanup_free_ char *address = NULL, *server_name = NULL; _cleanup_close_ int bus_ref = -1; const char *unique; cpu_set_t cpuset; diff --git a/src/login/logind-session.c b/src/login/logind-session.c index 730acd2978..db4dc45e05 100644 --- a/src/login/logind-session.c +++ b/src/login/logind-session.c @@ -1136,7 +1136,6 @@ void session_restore_vt(Session *s) { .mode = VT_AUTO, }; - _cleanup_free_ char *utf8 = NULL; int vt, old_fd; /* We need to get a fresh handle to the virtual terminal, diff --git a/src/network/networkd-routing-policy-rule.c b/src/network/networkd-routing-policy-rule.c index 6850135fc1..d37f31a4cf 100644 --- a/src/network/networkd-routing-policy-rule.c +++ b/src/network/networkd-routing-policy-rule.c @@ -672,7 +672,6 @@ int config_parse_routing_policy_rule_fwmark_mask( void *userdata) { _cleanup_routing_policy_rule_free_ RoutingPolicyRule *n = NULL; - _cleanup_free_ char *fwmark = NULL; Network *network = userdata; int r; @@ -751,7 +750,7 @@ int config_parse_routing_policy_rule_prefix( } static int routing_policy_rule_read_full_file(char *state_file, char **ret) { - _cleanup_free_ char *s = NULL, *p = NULL; + _cleanup_free_ char *s = NULL; size_t size; int r; diff --git a/src/shared/path-lookup.c b/src/shared/path-lookup.c index 802e4b9c8d..26dbb482ef 100644 --- a/src/shared/path-lookup.c +++ b/src/shared/path-lookup.c @@ -245,7 +245,6 @@ static int acquire_generator_dirs( char **generator_early, char **generator_late) { - _cleanup_(rmdir_and_freep) char *t = NULL; _cleanup_free_ char *x = NULL, *y = NULL, *z = NULL; const char *prefix; diff --git a/src/test/test-fileio.c b/src/test/test-fileio.c index 27495fa5be..c78eb51b42 100644 --- a/src/test/test-fileio.c +++ b/src/test/test-fileio.c @@ -704,7 +704,6 @@ static void test_read_line_one_file(FILE *f) { static void test_read_line(void) { _cleanup_fclose_ FILE *f = NULL; - _cleanup_free_ char *line = NULL; f = fmemopen((void*) buffer, sizeof(buffer), "re"); assert_se(f); From 1d3e682e124c283be367a4db55b73d491eeb48dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 1 Nov 2017 22:33:26 +0100 Subject: [PATCH 2/7] journald: unitialized variable access ../src/journal/journald-native.c:341:13: warning: variable 'context' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized] if (ucred && pid_is_valid(ucred->pid)) { ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ../src/journal/journald-native.c:350:42: note: uninitialized use occurs here context, ucred, tv, label, label_len); ^~~~~~~ ../src/journal/journald-native.c:335:31: note: initialize the variable 'context' to silence this warning ClientContext *context; ^ = NULL Very nice reporting! Functions that we call can handle context == NULL, so it's enough to simply initialize the variable. --- src/journal/journald-native.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/journal/journald-native.c b/src/journal/journald-native.c index d45c9c2270..1ff4e29849 100644 --- a/src/journal/journald-native.c +++ b/src/journal/journald-native.c @@ -332,7 +332,7 @@ void server_process_native_message( const char *label, size_t label_len) { size_t remaining = buffer_size; - ClientContext *context; + ClientContext *context = NULL; int r; assert(s); From 3dad3203ab872eef263f05bdf551384e91b619ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 1 Nov 2017 22:36:02 +0100 Subject: [PATCH 3/7] importd: remove IN_SET to avoid ambiguity clang warns: ../src/import/importd.c:254:70: warning: 'break' is bound to current loop, GCC binds it to the enclosing loop [-Wgcc-compat] while ((e < t->log_message + t->log_message_size) && IN_SET(*e, 0, '\n')) ^ Let's just play it safe and not use IN_SET here. --- src/import/importd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/import/importd.c b/src/import/importd.c index 22ac5fcc84..e23d6d0c80 100644 --- a/src/import/importd.c +++ b/src/import/importd.c @@ -251,7 +251,7 @@ static void transfer_send_logs(Transfer *t, bool flush) { n = strndup(t->log_message, e - t->log_message); /* Skip over NUL and newlines */ - while ((e < t->log_message + t->log_message_size) && IN_SET(*e, 0, '\n')) + while (e < t->log_message + t->log_message_size && (*e == 0 || *e == '\n')) e++; memmove(t->log_message, e, t->log_message + sizeof(t->log_message) - e); From ecc3f340abda85325df862ab2da612a78bf34926 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 1 Nov 2017 22:43:32 +0100 Subject: [PATCH 4/7] networkd: fix two format string mismatches ../src/network/networkd-link.c:3577:84: warning: format specifies type 'unsigned char' but the argument has type 'uint32_t' (aka 'unsigned int') [-Wformat] route->dst_prefixlen, route->tos, route->priority, route->table, route->lifetime); ^~~~~~~~~~~~ ../src/network/networkd-manager.c:1146:132: warning: format specifies type 'unsigned char' but the argument has type 'uint32_t' (aka 'unsigned int') [-Wformat] rule->from_prefixlen, space ? " " : "", to_str, rule->to_prefixlen, rule->tos, rule->fwmark, rule->fwmask, rule->table); ^~~~~~~~~~~ Also add some line breaks to make it easier to see which argument is for which part of the format string. --- src/network/networkd-link.c | 3 ++- src/network/networkd-manager.c | 8 ++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 70039940bc..fd063e43d2 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -3573,7 +3573,8 @@ int link_save(Link *link) { if (r < 0) goto fail; - fprintf(f, "%s%s/%hhu/%hhu/%"PRIu32"/%hhu/"USEC_FMT, space ? " " : "", route_str, + fprintf(f, "%s%s/%hhu/%hhu/%"PRIu32"/%"PRIu32"/"USEC_FMT, + space ? " " : "", route_str, route->dst_prefixlen, route->tos, route->priority, route->table, route->lifetime); space = true; } diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c index 200d0f622d..574bd0f5cb 100644 --- a/src/network/networkd-manager.c +++ b/src/network/networkd-manager.c @@ -1142,8 +1142,12 @@ static int manager_save(Manager *m) { goto fail; } - fprintf(f, "from=%s%s/%hhu to=%s%s/%hhu tos=%hhu fwmark=%"PRIu32"/%"PRIu32" table=%hhu", space ? " " : "", from_str, - rule->from_prefixlen, space ? " " : "", to_str, rule->to_prefixlen, rule->tos, rule->fwmark, rule->fwmask, rule->table); + fprintf(f, "from=%s%s/%hhu to=%s%s/%hhu tos=%hhu fwmark=%"PRIu32"/%"PRIu32" table=%"PRIu32, + space ? " " : "", from_str, rule->from_prefixlen, + space ? " " : "", to_str, rule->to_prefixlen, + rule->tos, + rule->fwmark, rule->fwmask, + rule->table); fputc('\n', f); } From 518044605173fee73f9e4b08a62624ae4b4f5bd2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 1 Nov 2017 22:54:39 +0100 Subject: [PATCH 5/7] journal: disable -Waddress-of-packed-member under clang clang warns about a few sites like this: ../src/journal/journal-file.c:1780:48: warning: taking address of packed member 'entry_offset' of class or structure 'DataObject' may result in an unaligned pointer value [-Waddress-of-packed-member] &o->data.entry_offset, ^~~~~~~~~~~~~~~~~~~~ but DataObject.entry_offset will always be 8-byte aligned as long as the DataObject structure is aligned. Similarly in other cases, the field is always aligned. Let's just silence the warning to avoid noise. gcc does not know -Waddress-of-packed-member, and would warn about an unknown warning, so we need to conditionalize on __clang__. --- src/journal/journal-file.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/journal/journal-file.c b/src/journal/journal-file.c index 3027801ab1..5d70421b76 100644 --- a/src/journal/journal-file.c +++ b/src/journal/journal-file.c @@ -90,6 +90,10 @@ /* The mmap context to use for the header we pick as one above the last defined typed */ #define CONTEXT_HEADER _OBJECT_TYPE_MAX +#ifdef __clang__ +# pragma GCC diagnostic ignored "-Waddress-of-packed-member" +#endif + /* This may be called from a separate thread to prevent blocking the caller for the duration of fsync(). * As a result we use atomic operations on f->offline_state for inter-thread communications with * journal_file_set_offline() and journal_file_set_online(). */ From 3f6914175c562e0193abfff9202bbd496daa1cae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 1 Nov 2017 23:01:24 +0100 Subject: [PATCH 6/7] util-lib: mark variable with _unused_ to silence clang warning _unused_ means "the variable is meant to be possible unused and gcc will not generate a warning about it", which is exactly what we need here, since we're only declaring it for the side effect of _cleanup_. --- src/basic/fileio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/basic/fileio.c b/src/basic/fileio.c index 9ee7a4af38..0ae330541d 100644 --- a/src/basic/fileio.c +++ b/src/basic/fileio.c @@ -1556,7 +1556,7 @@ int read_line(FILE *f, size_t limit, char **ret) { } { - _cleanup_(funlockfilep) FILE *flocked = f; + _unused_ _cleanup_(funlockfilep) FILE *flocked = f; flockfile(f); for (;;) { From 09571e3ffc6ee623739273975e4756d9b8e29cfe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 1 Nov 2017 23:06:27 +0100 Subject: [PATCH 7/7] test-util: silence clang warning about unaligned access --- src/test/test-util.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/test/test-util.c b/src/test/test-util.c index f8bf0cb875..0a0cb7b55e 100644 --- a/src/test/test-util.c +++ b/src/test/test-util.c @@ -104,6 +104,11 @@ static void test_max(void) { assert_se(CLAMP(CLAMP(0, -10, 10), CLAMP(-5, 10, 20), CLAMP(100, -5, 20)) == 10); } +#pragma GCC diagnostic push +#ifdef __clang__ +# pragma GCC diagnostic ignored "-Waddress-of-packed-member" +#endif + static void test_container_of(void) { struct mytype { uint8_t pad1[3]; @@ -122,6 +127,8 @@ static void test_container_of(void) { v1) == &myval); } +#pragma GCC diagnostic pop + static void test_div_round_up(void) { int div;