From 23450c897d11ccd8dfbe28cf3acca17f016e65be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 9 May 2020 08:53:27 +0200 Subject: [PATCH 1/2] core: fix compilation with gcc -O3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ../src/core/path.c: In function ‘path_serialize’: ../src/core/path.c:616:24: error: ‘%s’ directive argument is null [-Werror=format-overflow=] 616 | (void) serialize_item_format(f, "path-spec", "%s %%i %%s", | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 617 | path_type_to_string(s->type) //, | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 618 | // s->previous_exists, | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 619 | // s->path | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 620 | ); | ~ In function ‘path_spec_dump’, inlined from ‘path_dump’ at ../src/core/path.c:392:17: ../src/core/path.c:226:9: error: ‘%s’ directive argument is null [-Werror=format-overflow=] 226 | fprintf(f, | ^~~~~~~~~~ 227 | "%s%s: %s\n", | ~~~~~~~~~~~~~ 228 | prefix, | ~~~~~~~ 229 | path_type_to_string(s->type), | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 230 | s->path); | ~~~~~~~~ s->type should be valid here, so let's just add an assert. For https://github.com/systemd/systemd/issues/6119#issuecomment-626073743. --- src/core/path.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/core/path.c b/src/core/path.c index c7907ce4bf..1bbf27c5c5 100644 --- a/src/core/path.c +++ b/src/core/path.c @@ -223,11 +223,10 @@ static void path_spec_mkdir(PathSpec *s, mode_t mode) { } static void path_spec_dump(PathSpec *s, FILE *f, const char *prefix) { - fprintf(f, - "%s%s: %s\n", - prefix, - path_type_to_string(s->type), - s->path); + const char *type; + + assert_se(type = path_type_to_string(s->type)); + fprintf(f, "%s%s: %s\n", prefix, type, s->path); } void path_spec_done(PathSpec *s) { @@ -607,14 +606,16 @@ static int path_serialize(Unit *u, FILE *f, FDSet *fds) { (void) serialize_item(f, "result", path_result_to_string(p->result)); LIST_FOREACH(spec, s, p->specs) { + const char *type; _cleanup_free_ char *escaped = NULL; escaped = cescape(s->path); if (!escaped) return log_oom(); + assert_se(type = path_type_to_string(s->type)); (void) serialize_item_format(f, "path-spec", "%s %i %s", - path_type_to_string(s->type), + type, s->previous_exists, s->path); } From 94c0c5b7eaa1529ff694318d3755181ba96ecefb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 9 May 2020 09:09:11 +0200 Subject: [PATCH 2/2] shared/ethtool-util: hush gcc warnings about array bounds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [127/1355] Compiling C object 'src/shared/5afaae1@@systemd-shared-245@sta/ethtool-util.c.o' ../src/shared/ethtool-util.c: In function ‘ethtool_get_permanent_macaddr’: ../src/shared/ethtool-util.c:260:60: warning: array subscript 5 is outside the bounds of an interior zero-length array ‘__u8[0]’ {aka ‘unsigned char[]’} [-Wzero-length-bounds] 260 | ret->ether_addr_octet[i] = epaddr.addr.data[i]; | ~~~~~~~~~~~~~~~~^~~ In file included from ../src/shared/ethtool-util.c:5: ../src/shared/linux/ethtool.h:704:7: note: while referencing ‘data’ 704 | __u8 data[0]; | ^~~~ ../src/shared/ethtool-util.c: In function ‘ethtool_set_features’: ../src/shared/ethtool-util.c:488:31: warning: array subscript 0 is outside the bounds of an interior zero-length array ‘__u32[0]’ {aka ‘unsigned int[]’} [-Wzero-length-bounds] 488 | len = buffer.info.data[0]; | ~~~~~~~~~~~~~~~~^~~ In file included from ../src/shared/ethtool-util.c:5: ../src/shared/linux/ethtool.h:631:8: note: while referencing ‘data’ 631 | __u32 data[0]; | ^~~~ The kernel should not define the length of the array, but it does. We can't fix that, so let's use a cast to avoid the warning. For https://github.com/systemd/systemd/issues/6119#issuecomment-626073743. v2: - use #pragma instead of a cast. It seems the cast only works in some cases, and gcc is "smart" enough to see beyond the cast. Unfortunately clang does not support this warning, so we need to do a config check whether to try to suppress. --- meson.build | 3 +++ src/shared/ethtool-util.c | 10 ++++++++++ 2 files changed, 13 insertions(+) diff --git a/meson.build b/meson.build index 4c997ab6f7..580d5126fa 100644 --- a/meson.build +++ b/meson.build @@ -411,6 +411,9 @@ add_project_arguments(cc.get_supported_arguments(basic_disabled_warnings), langu add_project_arguments(cc.get_supported_arguments(possible_cc_flags), language : 'c') add_project_link_arguments(cc.get_supported_link_arguments(possible_link_flags), language : 'c') +have = cc.has_argument('-Wzero-length-bounds') +conf.set10('HAVE_ZERO_LENGTH_BOUNDS', have) + if cc.compiles(''' #include #include diff --git a/src/shared/ethtool-util.c b/src/shared/ethtool-util.c index 0cde87f5ac..25bc79eeea 100644 --- a/src/shared/ethtool-util.c +++ b/src/shared/ethtool-util.c @@ -256,8 +256,13 @@ int ethtool_get_permanent_macaddr(int *ethtool_fd, const char *ifname, struct et if (epaddr.addr.size != 6) return -EOPNOTSUPP; +#pragma GCC diagnostic push +#if HAVE_ZERO_LENGTH_BOUNDS +# pragma GCC diagnostic ignored "-Wzero-length-bounds" +#endif for (size_t i = 0; i < epaddr.addr.size; i++) ret->ether_addr_octet[i] = epaddr.addr.data[i]; +#pragma GCC diagnostic pop return 0; } @@ -485,7 +490,12 @@ static int get_stringset(int ethtool_fd, struct ifreq *ifr, int stringset_id, st if (!buffer.info.sset_mask) return -EINVAL; +#pragma GCC diagnostic push +#if HAVE_ZERO_LENGTH_BOUNDS +# pragma GCC diagnostic ignored "-Wzero-length-bounds" +#endif len = buffer.info.data[0]; +#pragma GCC diagnostic pop strings = malloc0(sizeof(struct ethtool_gstrings) + len * ETH_GSTRING_LEN); if (!strings)