From adf6d848df6e1fe51fc4985bef1f2744804ba052 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 29 Mar 2021 20:58:44 +0200 Subject: [PATCH 01/17] resolved: drop unnecessary {} --- src/resolve/resolved-dns-query.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/resolve/resolved-dns-query.c b/src/resolve/resolved-dns-query.c index fce676e02a3..ce58680cf6b 100644 --- a/src/resolve/resolved-dns-query.c +++ b/src/resolve/resolved-dns-query.c @@ -183,7 +183,7 @@ static DnsTransactionState dns_query_candidate_state(DnsQueryCandidate *c) { if (c->error_code != 0) return DNS_TRANSACTION_ERRNO; - SET_FOREACH(t, c->transactions) { + SET_FOREACH(t, c->transactions) switch (t->state) { @@ -213,7 +213,6 @@ static DnsTransactionState dns_query_candidate_state(DnsQueryCandidate *c) { break; } - } return state; } From befab2c40cc2c48ce8b8e28f93a6167e23b384e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 31 Mar 2021 11:06:41 +0200 Subject: [PATCH 02/17] basic/socket-util: add hint to silence gcc's maybe-unitialized warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [59/1551] Compiling C object src/basic/libbasic.a.p/socket-util.c.o ../src/basic/socket-util.c: In function ‘socket_get_mtu’: ../src/basic/socket-util.c:1393:16: warning: ‘mtu’ may be used uninitialized in this function [-Wmaybe-uninitialized] 1393 | *ret = (size_t) mtu; | ^~~~~~~~~~~~ --- src/basic/socket-util.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/basic/socket-util.h b/src/basic/socket-util.h index 507a599d7cf..e0b959f5da5 100644 --- a/src/basic/socket-util.h +++ b/src/basic/socket-util.h @@ -14,6 +14,7 @@ #include #include +#include "errno-util.h" #include "macro.h" #include "missing_network.h" #include "missing_socket.h" @@ -264,7 +265,7 @@ static inline int getsockopt_int(int fd, int level, int optname, int *ret) { socklen_t sl = sizeof(v); if (getsockopt(fd, level, optname, &v, &sl) < 0) - return -errno; + return negative_errno(); if (sl != sizeof(v)) return -EIO; From 55e2cfc9387ddf36fe115ae445b9a58fd5d3b89c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 31 Mar 2021 11:09:39 +0200 Subject: [PATCH 03/17] basic/fileio: silence gcc's maybe-unitialized warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [11/657] Compiling C object src/basic/libbasic.a.p/fileio.c.o ../src/basic/fileio.c: In function ‘write_string_stream_ts’: ../src/basic/fileio.c:167:21: warning: ‘fd’ may be used uninitialized in this function [-Wmaybe-uninitialized] 167 | if (futimens(fd, twice) < 0) | ^~~~~~~~~~~~~~~~~~~ --- src/basic/fileio.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/basic/fileio.c b/src/basic/fileio.c index df30870a1a2..abd822796e7 100644 --- a/src/basic/fileio.c +++ b/src/basic/fileio.c @@ -121,7 +121,7 @@ int write_string_stream_ts( const struct timespec *ts) { bool needs_nl; - int r, fd; + int r, fd = -1; assert(f); assert(line); @@ -140,8 +140,8 @@ int write_string_stream_ts( needs_nl = !(flags & WRITE_STRING_FILE_AVOID_NEWLINE) && !endswith(line, "\n"); if (needs_nl && (flags & WRITE_STRING_FILE_DISABLE_BUFFER)) { - /* If STDIO buffering was disabled, then let's append the newline character to the string itself, so - * that the write goes out in one go, instead of two */ + /* If STDIO buffering was disabled, then let's append the newline character to the string + * itself, so that the write goes out in one go, instead of two */ line = strjoina(line, "\n"); needs_nl = false; @@ -164,6 +164,7 @@ int write_string_stream_ts( if (ts) { const struct timespec twice[2] = {*ts, *ts}; + assert(fd >= 0); if (futimens(fd, twice) < 0) return -errno; } From c7e964c9444ecaaf52c5cd487e3331ba58a09f78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 31 Mar 2021 11:27:15 +0200 Subject: [PATCH 04/17] sd-bus: add assert to tell the compiler that the error code is positive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I was hoping it would help with the following gcc warning: [35/657] Compiling C object src/shared/libsystemd-shared-248.a.p/bus-message-util.c.o ../src/shared/bus-message-util.c: In function ‘bus_message_read_dns_servers’: ../src/shared/bus-message-util.c:165:21: warning: ‘family’ may be used uninitialized in this function [-Wmaybe-uninitialized] 165 | r = in_addr_full_new(family, &a, port, 0, server_name, dns + n); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ../src/shared/bus-message-util.c:165:21: warning: ‘port’ may be used uninitialized in this function [-Wmaybe-uninitialized] ../src/shared/bus-message-util.c:165:21: warning: ‘server_name’ may be used uninitialized in this function [-Wmaybe-uninitialized] It actually doesn't, but the compiler has a point here: the code is specified in sd_bus_error_map[], and it has no way of knowning that we want it to be a positive value. I think this should be an assert, because if this assumption fails, a programming error has occured, something that'd want to catch. --- src/libsystemd/sd-bus/bus-error.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/libsystemd/sd-bus/bus-error.c b/src/libsystemd/sd-bus/bus-error.c index 8da2024a502..163cbb1a71d 100644 --- a/src/libsystemd/sd-bus/bus-error.c +++ b/src/libsystemd/sd-bus/bus-error.c @@ -86,8 +86,10 @@ static int bus_error_name_to_errno(const char *name) { if (m->code == BUS_ERROR_MAP_END_MARKER) break; - if (streq(m->name, name)) + if (streq(m->name, name)) { + assert(m->code > 0); return m->code; + } } m = ALIGN_TO_PTR(__start_SYSTEMD_BUS_ERROR_MAP, sizeof(void*)); @@ -103,8 +105,10 @@ static int bus_error_name_to_errno(const char *name) { continue; } - if (streq(m->name, name)) + if (streq(m->name, name)) { + assert(m->code > 0); return m->code; + } m++; } From b9c19bc384fd41c173a8e453bd157544400af059 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 31 Mar 2021 11:45:15 +0200 Subject: [PATCH 05/17] sd-bus: add asserts showing that sd_bus_error_setf() returns negative (when name is specified). --- src/libsystemd/sd-bus/bus-error.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/libsystemd/sd-bus/bus-error.c b/src/libsystemd/sd-bus/bus-error.c index 163cbb1a71d..df292fe3d59 100644 --- a/src/libsystemd/sd-bus/bus-error.c +++ b/src/libsystemd/sd-bus/bus-error.c @@ -236,6 +236,7 @@ finish: } int bus_error_setfv(sd_bus_error *e, const char *name, const char *format, va_list ap) { + int r; if (!name) return 0; @@ -257,23 +258,28 @@ int bus_error_setfv(sd_bus_error *e, const char *name, const char *format, va_li e->_need_free = 1; } - return -bus_error_name_to_errno(name); + r = bus_error_name_to_errno(name); + assert(r > 0); + return -r; } _public_ int sd_bus_error_setf(sd_bus_error *e, const char *name, const char *format, ...) { + int r; if (format) { - int r; va_list ap; va_start(ap, format); r = bus_error_setfv(e, name, format, ap); + assert(!name || r < 0); va_end(ap); return r; } - return sd_bus_error_set(e, name, NULL); + r = sd_bus_error_set(e, name, NULL); + assert(!name || r < 0); + return r; } _public_ int sd_bus_error_copy(sd_bus_error *dest, const sd_bus_error *e) { From af46237ea1ba0d500ec2c8fea2e8501484491f60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 31 Mar 2021 11:46:31 +0200 Subject: [PATCH 06/17] man: split the description of sd_bus_error_set() It was one giant all of text in pseudo-random order. Let's split it into paragraphs talk about one subject each. And unfortunately, the description of what happens when the error is not set was not correct. In general, various functions treat 0/NULL as not-an-error, and return 0. --- man/sd_bus_error.xml | 197 ++++++++++++++---------------- src/libsystemd/sd-bus/bus-error.c | 2 +- 2 files changed, 93 insertions(+), 106 deletions(-) diff --git a/man/sd_bus_error.xml b/man/sd_bus_error.xml index bfc5caf4e24..bb8015de5ee 100644 --- a/man/sd_bus_error.xml +++ b/man/sd_bus_error.xml @@ -147,55 +147,54 @@ Description - The sd_bus_error structure carries - information about a D-Bus error condition. The functions described - below may be used to set and query fields in this structure. The - name field contains a short identifier - of an error. It should follow the rules for error names described - in the D-Bus specification, subsection Valid - Names. A number of common, standardized error names are - described in - sd-bus-errors3, - but additional domain-specific errors may be defined by - applications. The message field usually - contains a human-readable string describing the details, but might - be NULL. An unset sd_bus_error structure - should have both fields initialized to NULL. Set an error - structure to SD_BUS_ERROR_NULL in order to - reset both fields to NULL. When no longer necessary, resources - held by the sd_bus_error structure should - be destroyed with sd_bus_error_free(). + The sd_bus_error structure carries information about a D-Bus error + condition, or lack thereof. The functions described below may be used to set and query fields in this + structure. + + The name field contains a short identifier of an error. It + should follow the rules for error names described in the D-Bus specification, subsection Valid + Names. A number of common, standardized error names are described in + sd-bus-errors3, but + additional domain-specific errors may be defined by applications. - sd_bus_error_set() sets an error - structure to the specified name and message strings. The strings - will be copied into internal, newly allocated memory. It is - essential to free the error structure again when it is not - required anymore (see above). The function will return an - errno-like negative value (see The message field usually contains a human-readable string + describing the details, but might be NULL. + + An unset sd_bus_error structure should have both fields initialized to + NULL, and signifies lack of an error, i.e. success. Assign + SD_BUS_ERROR_NULL to the structure in order to initialize both fields to + NULL. When no longer necessary, resources held by the + sd_bus_error structure should be destroyed with + sd_bus_error_free(). + + sd_bus_error_set() sets an error structure to the specified name and message + strings. The strings will be copied into internal, newly allocated memory. It is essential to free the + contents again when they are not required anymore (see above). Do not use this call on error structures + that have already been set. If you intend to reuse an error structure, free the old data stored in it + with sd_bus_error_free() first. + + sd_bus_error_set() will return an errno-like value (see + errno3) - determined from the specified error name. Various well-known - D-Bus errors are converted to well-known errno - counterparts, and the other ones to -EIO. See - sd-bus-errors3 - for a list of well-known error names. Additional error mappings - may be defined with - sd_bus_error_add_map3. If - e is NULL, no error structure is initialized, - but the error is still converted into an - errno-style error. If - name is NULL, it is - assumed that no error occurred, and 0 is returned. This means that - this function may be conveniently used in a - return statement. If - message is NULL, no message is set. This - call can fail if no memory may be allocated for the name and - message strings, in which case an - SD_BUS_ERROR_NO_MEMORY error might be set - instead and -ENOMEM be returned. Do not use this call on error - structures that are already initialized. If you intend to reuse an - error structure, free the old data stored in it with - sd_bus_error_free() first. + determined from the specified error name name. If name is + NULL, it is assumed that no error occurred, and 0 is returned. + If name is nonnull, a negative value is always returned. If + e is NULL, no error structure is initialized, but + name is still converted into an errno-style value. + + Various well-known D-Bus errors are converted to well-known errno counterparts, + and the other ones to -EIO. See + sd-bus-errors3 for a + list of well-known error names. Additional error mappings may be defined with + sd_bus_error_add_map3. + + + sd_bus_error_set() is designed to be conveniently used in a + return statement. If message is NULL, no + message is set. This call can fail if no memory may be allocated for the name and message strings, in + which case an SD_BUS_ERROR_NO_MEMORY error will be set instead and + -ENOMEM returned. sd_bus_error_setf() is similar to sd_bus_error_set(), but takes a message field. sd_bus_error_set_const() is similar to - sd_bus_error_set(), but the string parameters - are not copied internally, and must hence remain constant and - valid for the lifetime of e. Use this call - to avoid memory allocations when setting error structures. Since - this call does not allocate memory, it will not fail with an - out-of-memory condition as - sd_bus_error_set() can, as described - above. Alternatively, the - SD_BUS_ERROR_MAKE_CONST() macro may be used - to generate a literal, constant bus error structure - on-the-fly. + sd_bus_error_set(), but the string parameters are not copied internally, and must + hence remain constant and valid for the lifetime of e. Use this call to avoid + memory allocations when setting error structures. Since this call does not allocate memory, it will not + fail with an out-of-memory condition as sd_bus_error_set() may, as described + above. Alternatively, the SD_BUS_ERROR_MAKE_CONST() macro may be used to generate a + literal, constant bus error structure on-the-fly. - sd_bus_error_set_errno() will set - name from an - errno-like value that is converted to a D-Bus + sd_bus_error_set_errno() will immediately return 0 if the + specified error parameter error is 0. Otherwise, it will set + name from an errno-like value that is converted to a D-Bus error. strerror_r3 - will be used to set message. Well-known - D-Bus error names will be used for name - if applicable, otherwise a name in the - System.Error. namespace will be generated. The - sign of the specified error number is ignored. The absolute value - is used implicitly. The call always returns a negative value, for - convenient usage in return statements. This - call might fail due to lack of memory, in which case an - SD_BUS_ERROR_NO_MEMORY error is set instead, - and -ENOMEM is returned. + project='die-net'>strerror_r3 will + be used to set message. Well-known D-Bus error names will be used for + name if applicable, otherwise a name in the System.Error. + namespace will be generated. The sign of the specified error number is ignored and the absolute value is + used implicitly. If the specified error error is non-zero, the call always returns + a negative value, for convenient usage in return statements. This call might fail + due to lack of memory, in which case an SD_BUS_ERROR_NO_MEMORY error is set instead, + and -ENOMEM is returned. sd_bus_error_set_errnof() is similar to sd_bus_error_set_errno(), but in addition to @@ -246,28 +237,26 @@ project='man-pages'>va_arg3 parameter list. - sd_bus_error_get_errno() converts the - name field of an error structure to an - errno-like (positive) value using the same - rules as sd_bus_error_set(). If - e is NULL, 0 will be - returned. + sd_bus_error_get_errno() converts the name field of + an error structure to an errno-like (positive) value using the same rules as + sd_bus_error_set(). If e is NULL, + 0 will be returned. - sd_bus_error_copy() will initialize - dst using the values in - e. If the strings in - e were set using - sd_bus_error_set_const(), they will be shared. - Otherwise, they will be copied. Returns a converted - errno-like, negative error code. + sd_bus_error_copy() will initialize dst using the + values in e, if e has been set with an error value before. + Otherwise, it will return immediately. If the strings in e were set using + sd_bus_error_set_const(), they will be shared. Otherwise, they will be + copied. Returns a converted errno-like, negative error code or 0. + Before this call, dst must be unset, i.e. either freshly initialized with + NULL or reset using sd_bus_error_free(). - sd_bus_error_move() is similar to sd_bus_error_copy(), but will - move any error information from e into dst, resetting the - former. This function cannot fail, as no new memory is allocated. Note that if e is not set - (or NULL) dst is initializated to - SD_BUS_ERROR_NULL. Moreover, if dst is NULL no - operation is executed on it and resources held by e are freed and reset. Returns a - converted errno-like, negative error code. + sd_bus_error_move() is similar to sd_bus_error_copy(), + but will move any error information from e into dst, + resetting the former. This function cannot fail, as no new memory is allocated. Note that if + e is not set, dst is initializated to + SD_BUS_ERROR_NULL. Moreover, if dst is + NULL no operation is executed on it and resources held by e + are freed and reset. Returns a converted errno-like, non-positive error value. sd_bus_error_is_set() will return a non-zero value if e is @@ -300,25 +289,23 @@ Return Value - The functions sd_bus_error_set(), - sd_bus_error_setf(), and - sd_bus_error_set_const(), when successful, - return the negative errno value corresponding to the - name parameter. The functions - sd_bus_error_set_errno(), - sd_bus_error_set_errnof() and - sd_bus_error_set_errnofv(), when successful, - return the negative value of the error - parameter. If an error occurs, one of the negative error values - listed below will be returned. + The functions sd_bus_error_set(), sd_bus_error_setf(), + and sd_bus_error_set_const() always return 0 when the specified + error value is NULL, and a negative errno-like value corresponding to the + name parameter otherwise. The functions + sd_bus_error_set_errno(), sd_bus_error_set_errnof() and + sd_bus_error_set_errnofv(), return 0 when the specified error + value is 0, and a a negative errno-like value corresponding to the + error parameter otherwise. If an error occurs internally, one of the negative + error values listed below will be returned. sd_bus_error_get_errno() returns false when e is NULL, and a positive errno value mapped from e->name otherwise. - sd_bus_error_copy() and sd_bus_error_move() return 0 or a positive - integer on success, and a negative error value converted from the error name otherwise. + sd_bus_error_copy() and sd_bus_error_move() return a + negative error value converted from the source error, and zero if the error has not been set. sd_bus_error_is_set() returns a non-zero value when e and the diff --git a/src/libsystemd/sd-bus/bus-error.c b/src/libsystemd/sd-bus/bus-error.c index df292fe3d59..7747600b837 100644 --- a/src/libsystemd/sd-bus/bus-error.c +++ b/src/libsystemd/sd-bus/bus-error.c @@ -465,7 +465,7 @@ _public_ int sd_bus_error_set_errno(sd_bus_error *e, int error) { if (!e) return -error; if (error == 0) - return -error; + return 0; assert_return(!bus_error_is_dirty(e), -EINVAL); From 4990d4b8ffc4fc866d558b7dba727e7fa4e36844 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 31 Mar 2021 12:43:00 +0200 Subject: [PATCH 07/17] varlink: use two local flag variables to silence gcc warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [59/655] Compiling C object src/shared/libsystemd-shared-248.a.p/varlink.c.o ../src/shared/varlink.c: In function ‘varlink_write’: ../src/shared/varlink.c:459:12: warning: ‘n’ may be used uninitialized in this function [-Wmaybe-uninitialized] 459 | if (n < 0) { | ^ ../src/shared/varlink.c: In function ‘varlink_process’: ../src/shared/varlink.c:541:12: warning: ‘n’ may be used uninitialized in this function [-Wmaybe-uninitialized] 541 | if (n < 0) { | ^ ../src/shared/varlink.c:486:17: note: ‘n’ was declared here 486 | ssize_t n; | ^ --- src/shared/varlink.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/shared/varlink.c b/src/shared/varlink.c index 6ed72075ba5..a271082ac33 100644 --- a/src/shared/varlink.c +++ b/src/shared/varlink.c @@ -448,13 +448,16 @@ static int varlink_write(Varlink *v) { assert(v->fd >= 0); /* We generally prefer recv()/send() (mostly because of MSG_NOSIGNAL) but also want to be compatible - * with non-socket IO, hence fall back automatically */ - if (!v->prefer_read_write) { + * with non-socket IO, hence fall back automatically. + * + * Use a local variable to help gcc figure out that we set 'n' in all cases. */ + bool prefer_write = v->prefer_read_write; + if (!prefer_write) { n = send(v->fd, v->output_buffer + v->output_buffer_index, v->output_buffer_size, MSG_DONTWAIT|MSG_NOSIGNAL); if (n < 0 && errno == ENOTSOCK) - v->prefer_read_write = true; + prefer_write = v->prefer_read_write = true; } - if (v->prefer_read_write) + if (prefer_write) n = write(v->fd, v->output_buffer + v->output_buffer_index, v->output_buffer_size); if (n < 0) { if (errno == EAGAIN) @@ -531,12 +534,13 @@ static int varlink_read(Varlink *v) { rs = v->input_buffer_allocated - (v->input_buffer_index + v->input_buffer_size); - if (!v->prefer_read_write) { + bool prefer_read = v->prefer_read_write; + if (!prefer_read) { n = recv(v->fd, v->input_buffer + v->input_buffer_index + v->input_buffer_size, rs, MSG_DONTWAIT); if (n < 0 && errno == ENOTSOCK) - v->prefer_read_write = true; + prefer_read = v->prefer_read_write = true; } - if (v->prefer_read_write) + if (prefer_read) n = read(v->fd, v->input_buffer + v->input_buffer_index + v->input_buffer_size, rs); if (n < 0) { if (errno == EAGAIN) From bfd9bfccceed07da65b3546ca47e7a53b7bf680a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 31 Mar 2021 14:06:39 +0200 Subject: [PATCH 08/17] sd-event: silence gcc's maybe-unitialized warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [91/180] Compiling C object libsystemd.a.p/src_libsystemd_sd-event_sd-event.c.o In file included from ../src/basic/macro.h:12, from ../src/basic/alloc-util.h:9, from ../src/libsystemd/sd-event/sd-event.c:11: ../src/libsystemd/sd-event/sd-event.c: In function ‘sd_event_wait’: ../src/fundamental/macro-fundamental.h:86:63: warning: ‘child_min_priority’ may be used uninitialized in this function [-Wmaybe-uninitialized] 86 | UNIQ_T(A, aq) < UNIQ_T(B, bq) ? UNIQ_T(A, aq) : UNIQ_T(B, bq); \ | ^ ../src/libsystemd/sd-event/sd-event.c:3983:45: note: ‘child_min_priority’ was declared here 3983 | int64_t epoll_min_priority, child_min_priority; | ^~~~~~~~~~~~~~~~~~ Alternative to #19159. --- src/libsystemd/sd-event/sd-event.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libsystemd/sd-event/sd-event.c b/src/libsystemd/sd-event/sd-event.c index b76b0623fe3..172be4e07e8 100644 --- a/src/libsystemd/sd-event/sd-event.c +++ b/src/libsystemd/sd-event/sd-event.c @@ -3171,7 +3171,7 @@ static int process_child(sd_event *e, int64_t threshold, int64_t *ret_min_priori zero(s->child.siginfo); if (waitid(P_PID, s->child.pid, &s->child.siginfo, WNOHANG | (s->child.options & WEXITED ? WNOWAIT : 0) | s->child.options) < 0) - return -errno; + return negative_errno(); if (s->child.siginfo.si_pid != 0) { bool zombie = IN_SET(s->child.siginfo.si_code, CLD_EXITED, CLD_KILLED, CLD_DUMPED); From bc20c31bbc249b419cf288fd640cd0466f098b04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 31 Mar 2021 14:21:21 +0200 Subject: [PATCH 09/17] basic/cgroup-util: silence gcc warning about unitialized variable --- src/basic/cgroup-util.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index 8dd3f8cd950..8d3e5237a8c 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -660,7 +660,7 @@ int cg_remove_xattr(const char *controller, const char *path, const char *name) int cg_pid_get_path(const char *controller, pid_t pid, char **ret_path) { _cleanup_fclose_ FILE *f = NULL; - const char *fs, *controller_str; + const char *fs, *controller_str = NULL; /* silence gcc warning about unitialized variable */ int unified, r; assert(pid >= 0); @@ -720,6 +720,7 @@ int cg_pid_get_path(const char *controller, pid_t pid, char **ret_path) { continue; *e = 0; + assert(controller_str); r = string_contains_word(l, ",", controller_str); if (r < 0) return r; From 0a94e77ed4cf5480f2ea0aac373392a9bb48571d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 31 Mar 2021 17:13:24 +0200 Subject: [PATCH 10/17] test-capability: silence gcc warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [2/3] Compiling C object test-capability.p/src_test_test-capability.c.o ../src/test/test-capability.c: In function ‘main’: ../src/test/test-capability.c:270:12: warning: ‘run_ambient’ may be used uninitialized in this function [-Wmaybe-uninitialized] 270 | if (run_ambient) | ^ gcc-11.0.1-0.3.fc34.x86_64 --- src/test/test-capability.c | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/src/test/test-capability.c b/src/test/test-capability.c index 0ff56070b16..bd17dc0e790 100644 --- a/src/test/test-capability.c +++ b/src/test/test-capability.c @@ -103,22 +103,15 @@ static int setup_tests(bool *run_ambient) { nobody = getpwnam(NOBODY_USER_NAME); if (!nobody) - return log_error_errno(SYNTHETIC_ERRNO(ENOENT), "Could not find nobody user: %m"); + return log_warning_errno(SYNTHETIC_ERRNO(ENOENT), "Couldn't find 'nobody' user: %m"); test_uid = nobody->pw_uid; test_gid = nobody->pw_gid; - *run_ambient = false; - r = prctl(PR_CAP_AMBIENT, PR_CAP_AMBIENT_CLEAR_ALL, 0, 0, 0); - - /* There's support for PR_CAP_AMBIENT if the prctl() call - * succeeded or error code was something else than EINVAL. The - * EINVAL check should be good enough to rule out false - * positives. */ - - if (r >= 0 || errno != EINVAL) - *run_ambient = true; + /* There's support for PR_CAP_AMBIENT if the prctl() call succeeded or error code was something else + * than EINVAL. The EINVAL check should be good enough to rule out false positives. */ + *run_ambient = r >= 0 || errno != EINVAL; return 0; } @@ -249,7 +242,7 @@ static void test_ensure_cap_64bit(void) { } int main(int argc, char *argv[]) { - bool run_ambient; + bool run_ambient = false; /* unnecessary initialization to silence gcc warning */ test_setup_logging(LOG_INFO); From 9fd8d678ba41ad39348758d5d329fe8d4451813f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 31 Mar 2021 17:24:26 +0200 Subject: [PATCH 11/17] shared/conf-parser: fix unitialized variable Introduced in 4f9ff96a55187927a4164a19df580329f4c6522b. --- src/shared/conf-parser.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/shared/conf-parser.c b/src/shared/conf-parser.c index 9dfa1907511..fa4079cff77 100644 --- a/src/shared/conf-parser.c +++ b/src/shared/conf-parser.c @@ -300,7 +300,8 @@ int config_parse( (void) stat_warn_permissions(filename, &st); mtime = timespec_load(&st.st_mtim); - } + } else + mtime = 0; for (;;) { _cleanup_free_ char *buf = NULL; From 487c123a326fb8e9ded95f9617eba38e906a1a14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 31 Mar 2021 17:32:04 +0200 Subject: [PATCH 12/17] shared/bus-message-util: silence gcc warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [1/429] Compiling C object src/shared/libsystemd-shared-248.a.p/bus-message-util.c.o ../src/shared/bus-message-util.c: In function ‘bus_message_read_dns_servers’: ../src/shared/bus-message-util.c:165:21: warning: ‘family’ may be used uninitialized in this function [-Wmaybe-uninitialized] 165 | r = in_addr_full_new(family, &a, port, 0, server_name, dns + n); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ../src/shared/bus-message-util.c:165:21: warning: ‘port’ may be used uninitialized in this function [-Wmaybe-uninitialized] ../src/shared/bus-message-util.c:165:21: warning: ‘server_name’ may be used uninitialized in this function [-Wmaybe-uninitialized] The warning would be there despite all the asserts in bus_error_setfv() and sd_bus_error_set(). So let's add an explicit assert. --- src/shared/bus-message-util.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/shared/bus-message-util.c b/src/shared/bus-message-util.c index 19500a552d7..565560c6be4 100644 --- a/src/shared/bus-message-util.c +++ b/src/shared/bus-message-util.c @@ -98,8 +98,11 @@ static int bus_message_read_dns_one( if (r < 0) return r; - if (!dns_server_address_valid(family, &a)) - return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid DNS server address"); + if (!dns_server_address_valid(family, &a)) { + r = sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid DNS server address"); + assert(r < 0); + return r; + } if (extended) { r = sd_bus_message_read(message, "q", &port); From 1a2948fece3a6b9f345a45be412e4622de288ff1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 31 Mar 2021 17:37:25 +0200 Subject: [PATCH 13/17] core: silence gcc warning --- src/core/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/main.c b/src/core/main.c index 0ddd6298513..74dd895c588 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -1367,7 +1367,7 @@ static int status_welcome(void) { static int write_container_id(void) { const char *c; - int r; + int r = 0; /* silence gcc warning about r being unitialized below */ c = getenv("container"); if (isempty(c)) From 1c93632ead00e04b76900966104c17419234dea1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 31 Mar 2021 17:46:04 +0200 Subject: [PATCH 14/17] shared/pretty-print: silence gcc warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit gcc was very unhappy for some reason: [988/1664] Compiling C object systemd-oomd.p/src_oom_oomd.c.o In file included from ../src/basic/path-util.h:10, from ../src/shared/pretty-print.c:14, from ../src/oom/oomd.c:15: ../src/shared/pretty-print.c: In function ‘conf_files_cat’: ../src/basic/strv.h:123:32: warning: ‘prefixes’ may be used uninitialized [-Wmaybe-uninitialized] 123 | for ((s) = (l); (s) && *(s); (s)++) | ^ In file included from ../src/oom/oomd.c:15: ../src/shared/pretty-print.c:283:16: note: ‘prefixes’ was declared here 283 | char **prefixes, **prefix; | ^~~~~~~~ ../src/shared/pretty-print.c:305:12: warning: ‘is_collection’ may be used uninitialized in this function [-Wmaybe-uninitialized] 305 | if (!is_collection) { | ^ ../src/shared/pretty-print.c:301:13: warning: ‘extension’ may be used uninitialized in this function [-Wmaybe-uninitialized] 301 | r = conf_files_list_strv(&files, extension, root, 0, (const char* const*) dirs); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Maybe this is caused by the statis char** variables? --- src/shared/pretty-print.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/shared/pretty-print.c b/src/shared/pretty-print.c index 0f02f327601..137ba77b3a6 100644 --- a/src/shared/pretty-print.c +++ b/src/shared/pretty-print.c @@ -280,7 +280,7 @@ static int guess_type(const char **name, char ***prefixes, bool *is_collection, int conf_files_cat(const char *root, const char *name) { _cleanup_strv_free_ char **dirs = NULL, **files = NULL; _cleanup_free_ char *path = NULL; - char **prefixes, **prefix; + char **prefix, **prefixes = NULL; /* explicit initialization to appease gcc */ bool is_collection; const char *extension; char **t; @@ -289,6 +289,8 @@ int conf_files_cat(const char *root, const char *name) { r = guess_type(&name, &prefixes, &is_collection, &extension); if (r < 0) return r; + assert(prefixes); + assert(extension); STRV_FOREACH(prefix, prefixes) { assert(endswith(*prefix, "/")); From 703e2870b1555ad95a164428c834154561288110 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 31 Mar 2021 18:07:25 +0200 Subject: [PATCH 15/17] systemctl: silence gcc maybe-unused warning --- src/systemctl/systemctl-start-unit.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/systemctl/systemctl-start-unit.c b/src/systemctl/systemctl-start-unit.c index c40e807212d..096b8ada200 100644 --- a/src/systemctl/systemctl-start-unit.c +++ b/src/systemctl/systemctl-start-unit.c @@ -307,7 +307,9 @@ int start_unit(int argc, char *argv[], void *userdata) { method = verb_to_method(argv[0]); job_type = verb_to_job_type(argv[0]); mode = arg_job_mode; - } + } else + method = job_type = mode = NULL; + one_name = NULL; } } else { From c26f7dd9f0098e7528619588bc940119d331001e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 31 Mar 2021 18:07:57 +0200 Subject: [PATCH 16/17] cryptsetup: silence gcc maybe-unused warning Simplify one debug stmt while at it. --- src/cryptsetup/cryptsetup.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/cryptsetup/cryptsetup.c b/src/cryptsetup/cryptsetup.c index 5c55dcd1975..ee31400e3f7 100644 --- a/src/cryptsetup/cryptsetup.c +++ b/src/cryptsetup/cryptsetup.c @@ -1008,7 +1008,7 @@ static int attach_luks_or_plain_or_bitlk_by_tpm2( _cleanup_(sd_event_unrefp) sd_event *event = NULL; _cleanup_free_ char *friendly = NULL; int keyslot = arg_key_slot, r; - size_t decrypted_key_size; + size_t decrypted_key_size = 0; /* Silence gcc warning about unitialized variable */ assert(cd); assert(name); @@ -1058,15 +1058,12 @@ static int attach_luks_or_plain_or_bitlk_by_tpm2( &policy_hash, &policy_hash_size, &keyslot, &token); - if (r == -ENXIO) { + if (r == -ENXIO) /* No further TPM2 tokens found in the LUKS2 header.*/ - if (found_some) - return log_debug_errno(SYNTHETIC_ERRNO(EAGAIN), - "No TPM2 metadata matching the current system state found in LUKS2 header, falling back to traditional unlocking."); - else - return log_debug_errno(SYNTHETIC_ERRNO(EAGAIN), - "No TPM2 metadata enrolled in LUKS2 header, falling back to traditional unlocking."); - } + return log_debug_errno(SYNTHETIC_ERRNO(EAGAIN), + found_some + ? "No TPM2 metadata matching the current system state found in LUKS2 header, falling back to traditional unlocking." + : "No TPM2 metadata enrolled in LUKS2 header, falling back to traditional unlocking."); if (r < 0) return r; @@ -1091,6 +1088,7 @@ static int attach_luks_or_plain_or_bitlk_by_tpm2( if (r != -EAGAIN) /* EAGAIN means: no tpm2 chip found */ return r; } + assert(decrypted_key); if (!monitor) { /* We didn't find the TPM2 device. In this case, watch for it via udev. Let's create From 1dbd0bdb3a8e80501b0c47a62f83ed28fd8311f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 31 Mar 2021 18:13:00 +0200 Subject: [PATCH 17/17] basic/env-util: silence two gcc warnings --- src/basic/env-util.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/basic/env-util.c b/src/basic/env-util.c index c110a750a55..0e8c2878d61 100644 --- a/src/basic/env-util.c +++ b/src/basic/env-util.c @@ -515,10 +515,10 @@ char *replace_env_n(const char *format, size_t n, char **env, unsigned flags) { ALTERNATE_VALUE, } state = WORD; - const char *e, *word = format, *test_value; + const char *e, *word = format, *test_value = NULL; /* test_value is initialized to appease gcc */ char *k; _cleanup_free_ char *r = NULL; - size_t i, len; + size_t i, len = 0; /* len is initialized to appease gcc */ int nest = 0; assert(format); @@ -581,13 +581,12 @@ char *replace_env_n(const char *format, size_t n, char **env, unsigned flags) { word = e+1; state = WORD; } else if (*e == ':') { - if (!(flags & REPLACE_ENV_ALLOW_EXTENDED)) + if (flags & REPLACE_ENV_ALLOW_EXTENDED) { + len = e - word - 2; + state = TEST; + } else /* Treat this as unsupported syntax, i.e. do no replacement */ state = WORD; - else { - len = e-word-2; - state = TEST; - } } break;