Merge pull request #19030 from dtardon/rhel9-covscan

a bunch of small fixes and clenups based on initial RHEL-9 covscan run

coccinelle/flags-set.cocci

@@ -8,7 +8,8 @@ position p : script:python() {
                  p[0].current_element == "log_set_max_level_realm" or
                  p[0].current_element == "unichar_is_valid")
         };
-expression x, y;
+expression x;
+constant y;
 @@
 (
 - ((x@p) & (y)) == (y)

coccinelle/run-coccinelle.sh

@@ -7,6 +7,7 @@ EXCLUDED_PATHS=(
     "src/basic/linux/*"
     # Symlinked to test-bus-vtable-cc.cc, which causes issues with the IN_SET macro
     "src/libsystemd/sd-bus/test-bus-vtable.c"
+    "src/libsystemd/sd-journal/lookup3.c"
 )
 
 top="$(git rev-parse --show-toplevel)"

coccinelle/strjoina.cocci

@@ -1,6 +1,7 @@
 @@
+position p : script:python() { p[0].current_element != "test_strjoina" };
 expression n, m;
 expression list s;
 @@
-- n = strjoina(m, s, NULL);
+- n = strjoina@p(m, s, NULL);
 + n = strjoina(m, s);

src/basic/fs-util.h

@@ -11,6 +11,7 @@
 #include <sys/types.h>
 #include <unistd.h>
 
+#include "alloc-util.h"
 #include "errno-util.h"
 #include "time-util.h"
 
@@ -107,8 +108,7 @@ static inline char *rmdir_and_free(char *p) {
                 return NULL;
 
         (void) rmdir(p);
-        free(p);
+        return mfree(p);
-        return NULL;
 }
 DEFINE_TRIVIAL_CLEANUP_FUNC(char*, rmdir_and_free);
 
@@ -117,8 +117,7 @@ static inline char* unlink_and_free(char *p) {
                 return NULL;
 
         (void) unlink_noerrno(p);
-        free(p);
+        return mfree(p);
-        return NULL;
 }
 DEFINE_TRIVIAL_CLEANUP_FUNC(char*, unlink_and_free);
 

src/basic/rm-rf.h

@@ -3,6 +3,7 @@
 
 #include <sys/stat.h>
 
+#include "alloc-util.h"
 #include "errno-util.h"
 
 typedef enum RemoveFlags {
@@ -25,8 +26,7 @@ static inline char *rm_rf_physical_and_free(char *p) {
                 return NULL;
 
         (void) rm_rf(p, REMOVE_ROOT|REMOVE_PHYSICAL|REMOVE_MISSING_OK|REMOVE_CHMOD);
-        free(p);
+        return mfree(p);
-        return NULL;
 }
 DEFINE_TRIVIAL_CLEANUP_FUNC(char*, rm_rf_physical_and_free);
 
@@ -38,7 +38,6 @@ static inline char *rm_rf_subvolume_and_free(char *p) {
                 return NULL;
 
         (void) rm_rf(p, REMOVE_ROOT|REMOVE_PHYSICAL|REMOVE_SUBVOLUME|REMOVE_MISSING_OK|REMOVE_CHMOD);
-        free(p);
+        return mfree(p);
-        return NULL;
 }
 DEFINE_TRIVIAL_CLEANUP_FUNC(char*, rm_rf_subvolume_and_free);

src/core/namespace.c

@@ -2051,13 +2051,12 @@ int setup_namespace(
                         };
                 }
 
-                if (ns_info->private_ipc) {
+                if (ns_info->private_ipc)
                         *(m++) = (MountEntry) {
                                 .path_const = "/dev/mqueue",
                                 .mode = MQUEUEFS,
                                 .flags = MS_NOSUID | MS_NODEV | MS_NOEXEC | MS_RELATIME,
                         };
-                }
 
                 if (creds_path) {
                         /* If our service has a credentials store configured, then bind that one in, but hide
@@ -2150,11 +2149,10 @@ int setup_namespace(
         if (setup_propagate)
                 (void) mkdir_p(propagate_dir, 0600);
 
-        if (n_extension_images > 0) {
+        if (n_extension_images > 0)
                 /* ExtensionImages mountpoint directories will be created
                  * while parsing the mounts to create, so have the parent ready */
                 (void) mkdir_p(extension_dir, 0600);
-        }
 
         /* Remount / as SLAVE so that nothing now mounted in the namespace
          * shows up in the parent */

src/journal-remote/journal-gatewayd.c

@@ -641,7 +641,7 @@ static int request_handler_redirect(
                 struct MHD_Connection *connection,
                 const char *target) {
 
-        char *page;
+        _cleanup_free_ char *page = NULL;
         _cleanup_(MHD_destroy_responsep) struct MHD_Response *response = NULL;
 
         assert(connection);
@@ -651,10 +651,9 @@ static int request_handler_redirect(
                 return respond_oom(connection);
 
         response = MHD_create_response_from_buffer(strlen(page), page, MHD_RESPMEM_MUST_FREE);
-        if (!response) {
+        if (!response)
-                free(page);
                 return respond_oom(connection);
-        }
+        TAKE_PTR(page);
 
         if (MHD_add_response_header(response, "Content-Type", "text/html") == MHD_NO ||
             MHD_add_response_header(response, "Location", target) == MHD_NO)

src/journal-remote/journal-upload.c

@@ -71,6 +71,9 @@ static void close_fd_input(Uploader *u);
                 }                                                       \
         } while (0)
 
+DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(CURL*, curl_easy_cleanup, NULL);
+DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(struct curl_slist*, curl_slist_free_all, NULL);
+
 static size_t output_callback(char *buf,
                               size_t size,
                               size_t nmemb,
@@ -180,29 +183,28 @@ int start_upload(Uploader *u,
         assert(input_callback);
 
         if (!u->header) {
-                struct curl_slist *h;
+                _cleanup_(curl_slist_free_allp) struct curl_slist *h = NULL;
+                struct curl_slist *l;
 
                 h = curl_slist_append(NULL, "Content-Type: application/vnd.fdo.journal");
                 if (!h)
                         return log_oom();
 
-                h = curl_slist_append(h, "Transfer-Encoding: chunked");
+                l = curl_slist_append(h, "Transfer-Encoding: chunked");
-                if (!h) {
+                if (!l)
-                        curl_slist_free_all(h);
                         return log_oom();
-                }
+                h = l;
 
-                h = curl_slist_append(h, "Accept: text/plain");
+                l = curl_slist_append(h, "Accept: text/plain");
-                if (!h) {
+                if (!l)
-                        curl_slist_free_all(h);
                         return log_oom();
-                }
+                h = l;
 
-                u->header = h;
+                u->header = TAKE_PTR(h);
         }
 
         if (!u->easy) {
-                CURL *curl;
+                _cleanup_(curl_easy_cleanupp) CURL *curl = NULL;
 
                 curl = curl_easy_init();
                 if (!curl)
@@ -260,7 +262,7 @@ int start_upload(Uploader *u,
                         easy_setopt(curl, CURLOPT_SSLVERSION, CURL_SSLVERSION_TLSv1,
                                     LOG_WARNING, );
 
-                u->easy = curl;
+                u->easy = TAKE_PTR(curl);
         } else {
                 /* truncate the potential old error message */
                 u->error[0] = '\0';

src/network/netdev/batadv.c

@@ -84,11 +84,10 @@ int config_parse_badadv_bandwidth (
                 return 0;
         }
 
-        if (k/1000/100 > UINT32_MAX) {
+        if (k/1000/100 > UINT32_MAX)
                 log_syntax(unit, LOG_WARNING, filename, line, 0,
                            "The value of '%s=', is outside of 0...429496729500000 range: %s",
                            lvalue, rvalue);
-        }
 
         *bandwidth = k/1000/100;
 

src/resolve/resolved-dns-rr.c

@@ -820,8 +820,8 @@ static char *format_txt(DnsTxtItem *first) {
 }
 
 const char *dns_resource_record_to_string(DnsResourceRecord *rr) {
-        _cleanup_free_ char *t = NULL;
+        _cleanup_free_ char *s = NULL, *t = NULL;
-        char *s, k[DNS_RESOURCE_KEY_STRING_MAX];
+        char k[DNS_RESOURCE_KEY_STRING_MAX];
         int r;
 
         assert(rr);
@@ -871,18 +871,15 @@ const char *dns_resource_record_to_string(DnsResourceRecord *rr) {
                         return NULL;
                 break;
 
-        case DNS_TYPE_A: {
+        case DNS_TYPE_A:
-                _cleanup_free_ char *x = NULL;
+                r = in_addr_to_string(AF_INET, (const union in_addr_union*) &rr->a.in_addr, &t);
-
-                r = in_addr_to_string(AF_INET, (const union in_addr_union*) &rr->a.in_addr, &x);
                 if (r < 0)
                         return NULL;
 
-                s = strjoin(k, " ", x);
+                s = strjoin(k, " ", t);
                 if (!s)
                         return NULL;
                 break;
-        }
 
         case DNS_TYPE_AAAA:
                 r = in_addr_to_string(AF_INET6, (const union in_addr_union*) &rr->aaaa.in6_addr, &t);
@@ -965,7 +962,6 @@ const char *dns_resource_record_to_string(DnsResourceRecord *rr) {
 
         case DNS_TYPE_DNSKEY: {
                 _cleanup_free_ char *alg = NULL;
-                char *ss;
                 uint16_t key_tag;
 
                 key_tag = dnssec_keytag(rr, true);
@@ -974,7 +970,7 @@ const char *dns_resource_record_to_string(DnsResourceRecord *rr) {
                 if (r < 0)
                         return NULL;
 
-                r = asprintf(&s, "%s %u %u %s",
+                r = asprintf(&t, "%s %u %u %s",
                              k,
                              rr->dnskey.flags,
                              rr->dnskey.protocol,
@@ -982,24 +978,22 @@ const char *dns_resource_record_to_string(DnsResourceRecord *rr) {
                 if (r < 0)
                         return NULL;
 
-                r = base64_append(&s, r,
+                r = base64_append(&t, r,
                                   rr->dnskey.key, rr->dnskey.key_size,
                                   8, columns());
                 if (r < 0)
                         return NULL;
 
-                r = asprintf(&ss, "%s\n"
+                r = asprintf(&s, "%s\n"
                              "        -- Flags:%s%s%s\n"
                              "        -- Key tag: %u",
-                             s,
+                             t,
                              rr->dnskey.flags & DNSKEY_FLAG_SEP ? " SEP" : "",
                              rr->dnskey.flags & DNSKEY_FLAG_REVOKE ? " REVOKE" : "",
                              rr->dnskey.flags & DNSKEY_FLAG_ZONE_KEY ? " ZONE_KEY" : "",
                              key_tag);
                 if (r < 0)
                         return NULL;
-                free(s);
-                s = ss;
 
                 break;
         }
@@ -1123,18 +1117,16 @@ const char *dns_resource_record_to_string(DnsResourceRecord *rr) {
                 break;
         }
 
-        case DNS_TYPE_CAA: {
+        case DNS_TYPE_CAA:
-                _cleanup_free_ char *value;
+                t = octescape(rr->caa.value, rr->caa.value_size);
-
+                if (!t)
-                value = octescape(rr->caa.value, rr->caa.value_size);
-                if (!value)
                         return NULL;
 
                 r = asprintf(&s, "%s %u %s \"%s\"%s%s%s%.0u",
                              k,
                              rr->caa.flags,
                              rr->caa.tag,
-                             value,
+                             t,
                              rr->caa.flags ? "\n        -- Flags:" : "",
                              rr->caa.flags & CAA_FLAG_CRITICAL ? " critical" : "",
                              rr->caa.flags & ~CAA_FLAG_CRITICAL ? " " : "",
@@ -1143,9 +1135,8 @@ const char *dns_resource_record_to_string(DnsResourceRecord *rr) {
                         return NULL;
 
                 break;
-        }
 
-        case DNS_TYPE_OPENPGPKEY: {
+        case DNS_TYPE_OPENPGPKEY:
                 r = asprintf(&s, "%s", k);
                 if (r < 0)
                         return NULL;
@@ -1156,7 +1147,6 @@ const char *dns_resource_record_to_string(DnsResourceRecord *rr) {
                 if (r < 0)
                         return NULL;
                 break;
-        }
 
         default:
                 t = hexmem(rr->generic.data, rr->generic.data_size);
@@ -1171,7 +1161,7 @@ const char *dns_resource_record_to_string(DnsResourceRecord *rr) {
         }
 
         rr->to_string = s;
-        return s;
+        return TAKE_PTR(s);
 }
 
 ssize_t dns_resource_record_payload(DnsResourceRecord *rr, void **out) {

src/shutdown/shutdown.c

@@ -487,7 +487,7 @@ int main(int argc, char *argv[]) {
                         /* There are things we cannot get rid of. Loop one more time
                          * with LOG_ERR to inform the user. Note that we don't need
                          * to do this if there is a initrd to switch to, because that
-                         * one is likely to get rid of the remounting mounts. If not,
+                         * one is likely to get rid of the remaining mounts. If not,
                          * it will log about them. */
                         umount_log_level = LOG_ERR;
                         continue;

src/systemctl/systemctl-edit.c

@@ -187,10 +187,12 @@ static int create_edit_temp_file(const char *new_path, const char *original_path
 
                         fprintf(f, "\n\n### %s", *path);
                         if (!isempty(contents)) {
-                                contents = strreplace(strstrip(contents), "\n", "\n# ");
+                                _cleanup_free_ char *commented_contents = NULL;
-                                if (!contents)
+
+                                commented_contents = strreplace(strstrip(contents), "\n", "\n# ");
+                                if (!commented_contents)
                                         return log_oom();
-                                fprintf(f, "\n# %s", contents);
+                                fprintf(f, "\n# %s", commented_contents);
                         }
                 }
 

src/test/test-copy.c

@@ -317,7 +317,7 @@ static void test_copy_proc(void) {
         assert_se(read_one_line_file("/proc/version", &a) >= 0);
         assert_se(read_one_line_file(f, &b) >= 0);
         assert_se(streq(a, b));
-        assert_se(strlen(a) > 0);
+        assert_se(!isempty(a));
 }
 
 int main(int argc, char *argv[]) {

src/veritysetup/veritysetup-generator.c

@@ -86,7 +86,7 @@ static int create_device(void) {
         if (r < 0)
                 return log_error_errno(r, "Failed to generate unit name: %m");
 
-        options_escaped = specifier_escape(arg_options ?: "");
+        options_escaped = specifier_escape(strempty(arg_options));
         if (!options_escaped)
                 return log_oom();