Merge pull request #8993 from keszybz/sd-resolve-coverity-and-related-fixes

sd-resolve coverity and related fixes

11 files changed, 117 insertions, 142 deletions

diff --git a/src/basic/path-util.c b/src/basic/path-util.c
index 460b7929bc..4f428818d1 100644
--- a/src/basic/path-util.c
+++ b/src/basic/path-util.c
@@ -919,7 +919,7 @@ int systemd_installation_has_version(const char *root, unsigned minimal_version)
                 if (r < 0)
                         return r;
 
-                assert_se((c = endswith(path, "*.so")));
+                assert_se(c = endswith(path, "*.so"));
                 *c = '\0'; /* truncate the glob part */
 
                 STRV_FOREACH(name, names) {
diff --git a/src/core/mount-setup.c b/src/core/mount-setup.c
index 79300332a8..e9d69728fa 100644
--- a/src/core/mount-setup.c
+++ b/src/core/mount-setup.c
@@ -317,7 +317,7 @@ int mount_cgroup_controllers(char ***join_controllers) {
                 if (r < 0)
                         return r;
 
-                if (r > 0 && k && *k) {
+                if (r > 0 && *k) {
                         char **i;
 
                         for (i = *k; *i; i++) {
diff --git a/src/libsystemd/sd-bus/test-bus-server.c b/src/libsystemd/sd-bus/test-bus-server.c
index f862dc06e8..373d596437 100644
--- a/src/libsystemd/sd-bus/test-bus-server.c
+++ b/src/libsystemd/sd-bus/test-bus-server.c
@@ -68,7 +68,8 @@ static void *server(void *p) {
 
                 if (sd_bus_message_is_method_call(m, "org.freedesktop.systemd.test", "Exit")) {
 
-                        assert_se((sd_bus_can_send(bus, 'h') >= 1) == (c->server_negotiate_unix_fds && c->client_negotiate_unix_fds));
+                        assert_se((sd_bus_can_send(bus, 'h') >= 1) ==
+                                  (c->server_negotiate_unix_fds && c->client_negotiate_unix_fds));
 
                         r = sd_bus_message_new_method_return(m, &reply);
                         if (r < 0) {
diff --git a/src/libsystemd/sd-resolve/sd-resolve.c b/src/libsystemd/sd-resolve/sd-resolve.c
index 4877206a2c..ba0f15538d 100644
--- a/src/libsystemd/sd-resolve/sd-resolve.c
+++ b/src/libsystemd/sd-resolve/sd-resolve.c
@@ -168,8 +168,10 @@ static void resolve_query_disconnect(sd_resolve_query *q);
 #define RESOLVE_DONT_DESTROY(resolve) \
         _cleanup_(sd_resolve_unrefp) _unused_ sd_resolve *_dont_destroy_##resolve = sd_resolve_ref(resolve)
 
-static int send_died(int out_fd) {
+#define ASSIGN_ERRNO(q, val, error, h_error) \
+        ({ (q)->ret = (val); (q)->_errno = (error); (q)->_h_errno = (h_error); })
 
+static int send_died(int out_fd) {
         RHeader rh = {
                 .type = RESPONSE_DIED,
                 .length = sizeof(RHeader),
@@ -231,12 +233,12 @@ static int send_addrinfo_reply(
                 ._h_errno = _h_errno,
         };
 
-        struct msghdr mh = {};
-        struct iovec iov[2];
         union {
                 AddrInfoSerialization ais;
                 uint8_t space[BUFSIZE];
         } buffer;
+        struct iovec iov[2];
+        struct msghdr mh;
 
         assert(out_fd >= 0);
 
@@ -259,8 +261,7 @@ static int send_addrinfo_reply(
         iov[0] = (struct iovec) { .iov_base = &resp, .iov_len = sizeof(AddrInfoResponse) };
         iov[1] = (struct iovec) { .iov_base = &buffer, .iov_len = resp.header.length - sizeof(AddrInfoResponse) };
 
-        mh.msg_iov = iov;
-        mh.msg_iovlen = ELEMENTSOF(iov);
+        mh = (struct msghdr) { .msg_iov = iov, .msg_iovlen = ELEMENTSOF(iov) };
 
         if (sendmsg(out_fd, &mh, MSG_NOSIGNAL) < 0)
                 return -errno;
@@ -285,8 +286,8 @@ static int send_nameinfo_reply(
                 ._h_errno = _h_errno,
         };
 
-        struct msghdr mh = {};
         struct iovec iov[3];
+        struct msghdr mh;
         size_t hl, sl;
 
         assert(out_fd >= 0);
@@ -302,8 +303,7 @@ static int send_nameinfo_reply(
         iov[1] = (struct iovec) { .iov_base = (void*) host, .iov_len = hl };
         iov[2] = (struct iovec) { .iov_base = (void*) serv, .iov_len = sl };
 
-        mh.msg_iov = iov;
-        mh.msg_iovlen = ELEMENTSOF(iov);
+        mh = (struct msghdr) { .msg_iov = iov, .msg_iovlen = ELEMENTSOF(iov) };
 
         if (sendmsg(out_fd, &mh, MSG_NOSIGNAL) < 0)
                 return -errno;
@@ -319,32 +319,33 @@ static int handle_request(int out_fd, const Packet *packet, size_t length) {
 
         req = &packet->rheader;
 
-        assert(length >= sizeof(RHeader));
-        assert(length == req->length);
+        assert_return(length >= sizeof(RHeader), -EIO);
+        assert_return(length == req->length, -EIO);
 
         switch (req->type) {
 
         case REQUEST_ADDRINFO: {
                const AddrInfoRequest *ai_req = &packet->addrinfo_request;
-               struct addrinfo hints = {}, *result = NULL;
+               struct addrinfo hints, *result = NULL;
                const char *node, *service;
                int ret;
 
-               assert(length >= sizeof(AddrInfoRequest));
-               assert(length == sizeof(AddrInfoRequest) + ai_req->node_len + ai_req->service_len);
+               assert_return(length >= sizeof(AddrInfoRequest), -EBADMSG);
+               assert_return(length == sizeof(AddrInfoRequest) + ai_req->node_len + ai_req->service_len, -EBADMSG);
 
-               hints.ai_flags = ai_req->ai_flags;
-               hints.ai_family = ai_req->ai_family;
-               hints.ai_socktype = ai_req->ai_socktype;
-               hints.ai_protocol = ai_req->ai_protocol;
+               hints = (struct addrinfo) {
+                       .ai_flags = ai_req->ai_flags,
+                       .ai_family = ai_req->ai_family,
+                       .ai_socktype = ai_req->ai_socktype,
+                       .ai_protocol = ai_req->ai_protocol,
+               };
 
                node = ai_req->node_len ? (const char*) ai_req + sizeof(AddrInfoRequest) : NULL;
                service = ai_req->service_len ? (const char*) ai_req + sizeof(AddrInfoRequest) + ai_req->node_len : NULL;
 
-               ret = getaddrinfo(
-                               node, service,
-                               ai_req->hints_valid ? &hints : NULL,
-                               &result);
+               ret = getaddrinfo(node, service,
+                                 ai_req->hints_valid ? &hints : NULL,
+                                 &result);
 
                /* send_addrinfo_reply() frees result */
                return send_addrinfo_reply(out_fd, req->id, ret, result, errno, h_errno);
@@ -356,21 +357,21 @@ static int handle_request(int out_fd, const Packet *packet, size_t length) {
                union sockaddr_union sa;
                int ret;
 
-               assert(length >= sizeof(NameInfoRequest));
-               assert(length == sizeof(NameInfoRequest) + ni_req->sockaddr_len);
-               assert(sizeof(sa) >= ni_req->sockaddr_len);
+               assert_return(length >= sizeof(NameInfoRequest), -EBADMSG);
+               assert_return(length == sizeof(NameInfoRequest) + ni_req->sockaddr_len, -EBADMSG);
+               assert_return(ni_req->sockaddr_len <= sizeof(sa), -EBADMSG);
 
                memcpy(&sa, (const uint8_t *) ni_req + sizeof(NameInfoRequest), ni_req->sockaddr_len);
 
                ret = getnameinfo(&sa.sa, ni_req->sockaddr_len,
-                               ni_req->gethost ? hostbuf : NULL, ni_req->gethost ? sizeof(hostbuf) : 0,
-                               ni_req->getserv ? servbuf : NULL, ni_req->getserv ? sizeof(servbuf) : 0,
-                               ni_req->flags);
+                                 ni_req->gethost ? hostbuf : NULL, ni_req->gethost ? sizeof(hostbuf) : 0,
+                                 ni_req->getserv ? servbuf : NULL, ni_req->getserv ? sizeof(servbuf) : 0,
+                                 ni_req->flags);
 
                return send_nameinfo_reply(out_fd, req->id, ret,
-                               ret == 0 && ni_req->gethost ? hostbuf : NULL,
-                               ret == 0 && ni_req->getserv ? servbuf : NULL,
-                               errno, h_errno);
+                                          ret == 0 && ni_req->gethost ? hostbuf : NULL,
+                                          ret == 0 && ni_req->getserv ? servbuf : NULL,
+                                          errno, h_errno);
         }
 
         case REQUEST_TERMINATE:
@@ -397,7 +398,7 @@ static void* thread_worker(void *p) {
                 } buf;
                 ssize_t length;
 
-                length = recv(resolve->fds[REQUEST_RECV_FD], &buf, sizeof(buf), 0);
+                length = recv(resolve->fds[REQUEST_RECV_FD], &buf, sizeof buf, 0);
                 if (length < 0) {
                         if (errno == EINTR)
                                 continue;
@@ -407,9 +408,6 @@ static void* thread_worker(void *p) {
                 if (length == 0)
                         break;
 
-                if (resolve->dead)
-                        break;
-
                 if (handle_request(resolve->fds[RESPONSE_SEND_FD], &buf.packet, (size_t) length) < 0)
                         break;
         }
@@ -437,7 +435,6 @@ static int start_threads(sd_resolve *resolve, unsigned extra) {
         n = CLAMP(n, WORKERS_MIN, WORKERS_MAX);
 
         while (resolve->n_valid_workers < n) {
-
                 r = pthread_create(&resolve->workers[resolve->n_valid_workers], NULL, thread_worker, resolve);
                 if (r > 0) {
                         r = -r;
@@ -467,8 +464,8 @@ static bool resolve_pid_changed(sd_resolve *r) {
 }
 
 _public_ int sd_resolve_new(sd_resolve **ret) {
-        sd_resolve *resolve = NULL;
-        int i, r;
+        _cleanup_(sd_resolve_unrefp) sd_resolve *resolve = NULL;
+        int i;
 
         assert_return(ret, -EINVAL);
 
@@ -482,17 +479,11 @@ _public_ int sd_resolve_new(sd_resolve **ret) {
         for (i = 0; i < _FD_MAX; i++)
                 resolve->fds[i] = -1;
 
-        r = socketpair(PF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC, 0, resolve->fds + REQUEST_RECV_FD);
-        if (r < 0) {
-                r = -errno;
-                goto fail;
-        }
+        if (socketpair(PF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC, 0, resolve->fds + REQUEST_RECV_FD) < 0)
+                return -errno;
 
-        r = socketpair(PF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC, 0, resolve->fds + RESPONSE_RECV_FD);
-        if (r < 0) {
-                r = -errno;
-                goto fail;
-        }
+        if (socketpair(PF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC, 0, resolve->fds + RESPONSE_RECV_FD) < 0)
+                return -errno;
 
         for (i = 0; i < _FD_MAX; i++)
                 resolve->fds[i] = fd_move_above_stdio(resolve->fds[i]);
@@ -504,12 +495,8 @@ _public_ int sd_resolve_new(sd_resolve **ret) {
 
         (void) fd_nonblock(resolve->fds[RESPONSE_RECV_FD], true);
 
-        *ret = resolve;
+        *ret = TAKE_PTR(resolve);
         return 0;
-
-fail:
-        sd_resolve_unref(resolve);
-        return r;
 }
 
 _public_ int sd_resolve_default(sd_resolve **ret) {
@@ -578,7 +565,7 @@ static void resolve_free(sd_resolve *resolve) {
 
                 RHeader req = {
                         .type = REQUEST_TERMINATE,
-                        .length = sizeof(req)
+                        .length = sizeof req,
                 };
 
                 /* Send one termination packet for each worker */
@@ -606,7 +593,6 @@ _public_ sd_resolve* sd_resolve_ref(sd_resolve *resolve) {
 }
 
 _public_ sd_resolve* sd_resolve_unref(sd_resolve *resolve) {
-
         if (!resolve)
                 return NULL;
 
@@ -752,11 +738,11 @@ static int handle_response(sd_resolve *resolve, const Packet *packet, size_t len
         int r;
 
         assert(resolve);
+        assert(packet);
 
         resp = &packet->rheader;
-        assert(resp);
-        assert(length >= sizeof(RHeader));
-        assert(length == resp->length);
+        assert_return(length >= sizeof(RHeader), -EIO);
+        assert_return(length == resp->length, -EIO);
 
         if (resp->type == RESPONSE_DIED) {
                 resolve->dead = true;
@@ -778,12 +764,10 @@ static int handle_response(sd_resolve *resolve, const Packet *packet, size_t len
                 size_t l;
                 struct addrinfo *prev = NULL;
 
-                assert(length >= sizeof(AddrInfoResponse));
-                assert(q->type == REQUEST_ADDRINFO);
+                assert_return(length >= sizeof(AddrInfoResponse), -EBADMSG);
+                assert_return(q->type == REQUEST_ADDRINFO, -EBADMSG);
 
-                q->ret = ai_resp->ret;
-                q->_errno = ai_resp->_errno;
-                q->_h_errno = ai_resp->_h_errno;
+                ASSIGN_ERRNO(q, ai_resp->ret, ai_resp->_errno, ai_resp->_h_errno);
 
                 l = length - sizeof(AddrInfoResponse);
                 p = (const uint8_t*) resp + sizeof(AddrInfoResponse);
@@ -793,9 +777,7 @@ static int handle_response(sd_resolve *resolve, const Packet *packet, size_t len
 
                         r = unserialize_addrinfo(&p, &l, &ai);
                         if (r < 0) {
-                                q->ret = EAI_SYSTEM;
-                                q->_errno = -r;
-                                q->_h_errno = 0;
+                                ASSIGN_ERRNO(q, EAI_SYSTEM, -r, 0);
                                 freeaddrinfo(q->addrinfo);
                                 q->addrinfo = NULL;
                                 break;
@@ -815,39 +797,29 @@ static int handle_response(sd_resolve *resolve, const Packet *packet, size_t len
         case RESPONSE_NAMEINFO: {
                 const NameInfoResponse *ni_resp = &packet->nameinfo_response;
 
-                assert(length >= sizeof(NameInfoResponse));
-                assert(q->type == REQUEST_NAMEINFO);
+                assert_return(length >= sizeof(NameInfoResponse), -EBADMSG);
+                assert_return(q->type == REQUEST_NAMEINFO, -EBADMSG);
 
                 if (ni_resp->hostlen > DNS_HOSTNAME_MAX ||
                     ni_resp->servlen > DNS_HOSTNAME_MAX ||
-                    sizeof(NameInfoResponse) + ni_resp->hostlen + ni_resp->servlen > length + 2) {
-                        q->ret = EAI_SYSTEM;
-                        q->_errno = -EIO;
-                        q->_h_errno = 0;
+                    sizeof(NameInfoResponse) + ni_resp->hostlen + ni_resp->servlen > length)
+                        ASSIGN_ERRNO(q, EAI_SYSTEM, EIO, 0);
 
-                } else {
-                        q->ret = ni_resp->ret;
-                        q->_errno = ni_resp->_errno;
-                        q->_h_errno = ni_resp->_h_errno;
+                else {
+                        ASSIGN_ERRNO(q, ni_resp->ret, ni_resp->_errno, ni_resp->_h_errno);
 
                         if (ni_resp->hostlen > 0) {
                                 q->host = strndup((const char*) ni_resp + sizeof(NameInfoResponse),
                                                   ni_resp->hostlen-1);
-                                if (!q->host) {
-                                        q->ret = EAI_MEMORY;
-                                        q->_errno = ENOMEM;
-                                        q->_h_errno = 0;
-                                }
+                                if (!q->host)
+                                        ASSIGN_ERRNO(q, EAI_MEMORY, ENOMEM, 0);
                         }
 
                         if (ni_resp->servlen > 0) {
                                 q->serv = strndup((const char*) ni_resp + sizeof(NameInfoResponse) + ni_resp->hostlen,
                                                   ni_resp->servlen-1);
-                                if (!q->serv) {
-                                        q->ret = EAI_MEMORY;
-                                        q->_errno = ENOMEM;
-                                        q->_h_errno = 0;
-                                }
+                                if (!q->serv)
+                                        ASSIGN_ERRNO(q, EAI_MEMORY, ENOMEM, 0);
                         }
                 }
 
@@ -875,7 +847,7 @@ _public_ int sd_resolve_process(sd_resolve *resolve) {
         /* We don't allow recursively invoking sd_resolve_process(). */
         assert_return(!resolve->current, -EBUSY);
 
-        l = recv(resolve->fds[RESPONSE_RECV_FD], &buf, sizeof(buf), 0);
+        l = recv(resolve->fds[RESPONSE_RECV_FD], &buf, sizeof buf, 0);
         if (l < 0) {
                 if (errno == EAGAIN)
                         return 0;
@@ -956,11 +928,12 @@ _public_ int sd_resolve_getaddrinfo(
                 const struct addrinfo *hints,
                 sd_resolve_getaddrinfo_handler_t callback, void *userdata) {
 
-        AddrInfoRequest req = {};
-        struct msghdr mh = {};
+        _cleanup_(sd_resolve_query_unrefp) sd_resolve_query *q = NULL;
+        AddrInfoRequest req;
         struct iovec iov[3];
-        sd_resolve_query *q;
+        struct msghdr mh = {};
         int r;
+        size_t node_len, service_len;
 
         assert_return(resolve, -EINVAL);
         assert_return(node || service, -EINVAL);
@@ -975,20 +948,23 @@ _public_ int sd_resolve_getaddrinfo(
         q->getaddrinfo_handler = callback;
         q->userdata = userdata;
 
-        req.node_len = node ? strlen(node)+1 : 0;
-        req.service_len = service ? strlen(service)+1 : 0;
+        node_len = node ? strlen(node) + 1 : 0;
+        service_len = service ? strlen(service) + 1 : 0;
 
-        req.header.id = q->id;
-        req.header.type = REQUEST_ADDRINFO;
-        req.header.length = sizeof(AddrInfoRequest) + req.node_len + req.service_len;
+        req = (AddrInfoRequest) {
+                .node_len = node_len,
+                .service_len = service_len,
 
-        if (hints) {
-                req.hints_valid = true;
-                req.ai_flags = hints->ai_flags;
-                req.ai_family = hints->ai_family;
-                req.ai_socktype = hints->ai_socktype;
-                req.ai_protocol = hints->ai_protocol;
-        }
+                .header.id = q->id,
+                .header.type = REQUEST_ADDRINFO,
+                .header.length = sizeof(AddrInfoRequest) + node_len + service_len,
+
+                .hints_valid = hints,
+                .ai_flags = hints ? hints->ai_flags : 0,
+                .ai_family = hints ? hints->ai_family : 0,
+                .ai_socktype = hints ? hints->ai_socktype : 0,
+                .ai_protocol = hints ? hints->ai_protocol : 0,
+        };
 
         iov[mh.msg_iovlen++] = (struct iovec) { .iov_base = &req, .iov_len = sizeof(AddrInfoRequest) };
         if (node)
@@ -997,15 +973,14 @@ _public_ int sd_resolve_getaddrinfo(
                 iov[mh.msg_iovlen++] = (struct iovec) { .iov_base = (void*) service, .iov_len = req.service_len };
         mh.msg_iov = iov;
 
-        if (sendmsg(resolve->fds[REQUEST_SEND_FD], &mh, MSG_NOSIGNAL) < 0) {
-                sd_resolve_query_unref(q);
+        if (sendmsg(resolve->fds[REQUEST_SEND_FD], &mh, MSG_NOSIGNAL) < 0)
                 return -errno;
-        }
 
         resolve->n_outstanding++;
 
         if (_q)
                 *_q = q;
+        TAKE_PTR(q);
 
         return 0;
 }
@@ -1030,10 +1005,10 @@ _public_ int sd_resolve_getnameinfo(
                 sd_resolve_getnameinfo_handler_t callback,
                 void *userdata) {
 
-        NameInfoRequest req = {};
-        struct msghdr mh = {};
+        _cleanup_(sd_resolve_query_unrefp) sd_resolve_query *q = NULL;
+        NameInfoRequest req;
         struct iovec iov[2];
-        sd_resolve_query *q;
+        struct msghdr mh;
         int r;
 
         assert_return(resolve, -EINVAL);
@@ -1052,31 +1027,31 @@ _public_ int sd_resolve_getnameinfo(
         q->getnameinfo_handler = callback;
         q->userdata = userdata;
 
-        req.header.id = q->id;
-        req.header.type = REQUEST_NAMEINFO;
-        req.header.length = sizeof(NameInfoRequest) + salen;
+        req = (NameInfoRequest) {
+                .header.id = q->id,
+                .header.type = REQUEST_NAMEINFO,
+                .header.length = sizeof(NameInfoRequest) + salen,
 
-        req.flags = flags;
-        req.sockaddr_len = salen;
-        req.gethost = !!(get & SD_RESOLVE_GET_HOST);
-        req.getserv = !!(get & SD_RESOLVE_GET_SERVICE);
+                .flags = flags,
+                .sockaddr_len = salen,
+                .gethost = !!(get & SD_RESOLVE_GET_HOST),
+                .getserv = !!(get & SD_RESOLVE_GET_SERVICE),
+        };
 
         iov[0] = (struct iovec) { .iov_base = &req, .iov_len = sizeof(NameInfoRequest) };
         iov[1] = (struct iovec) { .iov_base = (void*) sa, .iov_len = salen };
 
-        mh.msg_iov = iov;
-        mh.msg_iovlen = 2;
+        mh = (struct msghdr) { .msg_iov = iov, .msg_iovlen = ELEMENTSOF(iov) };
 
-        if (sendmsg(resolve->fds[REQUEST_SEND_FD], &mh, MSG_NOSIGNAL) < 0) {
-                sd_resolve_query_unref(q);
+        if (sendmsg(resolve->fds[REQUEST_SEND_FD], &mh, MSG_NOSIGNAL) < 0)
                 return -errno;
-        }
-
-        resolve->n_outstanding++;
 
         if (_q)
                 *_q = q;
 
+        resolve->n_outstanding++;
+        TAKE_PTR(q);
+
         return 0;
 }
 
@@ -1087,7 +1062,7 @@ static int getnameinfo_done(sd_resolve_query *q) {
         assert(q->getnameinfo_handler);
 
         errno = q->_errno;
-        h_errno= q->_h_errno;
+        h_errno = q->_h_errno;
 
         return q->getnameinfo_handler(q, q->ret, q->host, q->serv, q->userdata);
 }
diff --git a/src/libsystemd/sd-resolve/test-resolve.c b/src/libsystemd/sd-resolve/test-resolve.c
index 5ec35e1b4f..7020412ab2 100644
--- a/src/libsystemd/sd-resolve/test-resolve.c
+++ b/src/libsystemd/sd-resolve/test-resolve.c
@@ -103,7 +103,6 @@ int main(int argc, char *argv[]) {
 
                         log_notice_errno(r, "sd_resolve_wait() timed out, but that's OK");
                         exit(EXIT_SUCCESS);
-                        break;
                 }
                 if (r < 0) {
                         log_error_errno(r, "sd_resolve_wait(): %m");
diff --git a/src/test/test-copy.c b/src/test/test-copy.c
index 9d6a6540c6..a02746c76d 100644
--- a/src/test/test-copy.c
+++ b/src/test/test-copy.c
@@ -95,7 +95,7 @@ static void test_copy_tree(void) {
         STRV_FOREACH(p, files) {
                 _cleanup_free_ char *f;
 
-                assert_se((f = strappend(original_dir, *p)));
+                assert_se(f = strappend(original_dir, *p));
 
                 assert_se(mkdir_parents(f, 0755) >= 0);
                 assert_se(write_string_file(f, "file", WRITE_STRING_FILE_CREATE) == 0);
@@ -104,8 +104,8 @@ static void test_copy_tree(void) {
         STRV_FOREACH_PAIR(link, p, links) {
                 _cleanup_free_ char *f, *l;
 
-                assert_se((f = strappend(original_dir, *p)));
-                assert_se((l = strappend(original_dir, *link)));
+                assert_se(f = strappend(original_dir, *p));
+                assert_se(l = strappend(original_dir, *link));
 
                 assert_se(mkdir_parents(l, 0755) >= 0);
                 assert_se(symlink(f, l) == 0);
@@ -117,10 +117,10 @@ static void test_copy_tree(void) {
         assert_se(copy_tree(original_dir, copy_dir, UID_INVALID, GID_INVALID, COPY_REFLINK|COPY_MERGE) == 0);
 
         STRV_FOREACH(p, files) {
-                _cleanup_free_ char *buf = NULL, *f;
-                size_t sz = 0;
+                _cleanup_free_ char *buf, *f;
+                size_t sz;
 
-                assert_se((f = strappend(copy_dir, *p)));
+                assert_se(f = strappend(copy_dir, *p));
 
                 assert_se(access(f, F_OK) == 0);
                 assert_se(read_full_file(f, &buf, &sz) == 0);
@@ -128,10 +128,10 @@ static void test_copy_tree(void) {
         }
 
         STRV_FOREACH_PAIR(link, p, links) {
-                _cleanup_free_ char *target = NULL, *f, *l;
+                _cleanup_free_ char *target, *f, *l;
 
-                assert_se((f = strjoin(original_dir, *p)));
-                assert_se((l = strjoin(copy_dir, *link)));
+                assert_se(f = strjoin(original_dir, *p));
+                assert_se(l = strjoin(copy_dir, *link));
 
                 assert_se(chase_symlinks(l, NULL, 0, &target) == 1);
                 assert_se(path_equal(f, target));
diff --git a/src/test/test-dlopen.c b/src/test/test-dlopen.c
index 151d2d72b2..febfc60e00 100644
--- a/src/test/test-dlopen.c
+++ b/src/test/test-dlopen.c
@@ -13,7 +13,7 @@
 int main(int argc, char **argv) {
         void *handle;
 
-        assert_se((handle = dlopen(argv[1], RTLD_NOW)));
+        assert_se(handle = dlopen(argv[1], RTLD_NOW));
         assert_se(dlclose(handle) == 0);
 
         return EXIT_SUCCESS;
diff --git a/src/test/test-fileio.c b/src/test/test-fileio.c
index 50d1e792c0..bbddbf3bcd 100644
--- a/src/test/test-fileio.c
+++ b/src/test/test-fileio.c
@@ -454,7 +454,7 @@ static void test_write_string_file_verify(void) {
         int r;
 
         assert_se(read_one_line_file("/proc/cmdline", &buf) >= 0);
-        assert_se((buf2 = strjoin(buf, "\n")));
+        assert_se(buf2 = strjoin(buf, "\n"));
 
         r = write_string_file("/proc/cmdline", buf, 0);
         assert_se(IN_SET(r, -EACCES, -EIO));
diff --git a/src/test/test-hexdecoct.c b/src/test/test-hexdecoct.c
index fb98826c27..41ca9805fe 100644
--- a/src/test/test-hexdecoct.c
+++ b/src/test/test-hexdecoct.c
@@ -85,10 +85,10 @@ static void test_unhexmem_one(const char *s, size_t l, int retval) {
         if (retval == 0) {
                 char *answer;
 
-                if (l == (size_t) - 1)
+                if (l == (size_t) -1)
                         l = strlen(s);
 
-                assert_se((hex = hexmem(mem, len)));
+                assert_se(hex = hexmem(mem, len));
                 answer = strndupa(s, l);
                 assert_se(streq(delete_chars(answer, WHITESPACE), hex));
         }
diff --git a/src/test/test-nss.c b/src/test/test-nss.c
index f6dee45ac0..6d4c51d6d8 100644
--- a/src/test/test-nss.c
+++ b/src/test/test-nss.c
@@ -87,7 +87,7 @@ static int print_gaih_addrtuples(const struct gaih_addrtuple *tuples) {
                 r = in_addr_to_string(it->family, &u, &a);
                 assert_se(IN_SET(r, 0, -EAFNOSUPPORT));
                 if (r == -EAFNOSUPPORT)
-                        assert_se((a = hexmem(it->addr, 16)));
+                        assert_se(a = hexmem(it->addr, 16));
 
                 if (it->scopeid == 0)
                         goto numerical_index;
diff --git a/src/test/test-unit-file.c b/src/test/test-unit-file.c
index fb4da53027..7b74ed0dc3 100644
--- a/src/test/test-unit-file.c
+++ b/src/test/test-unit-file.c
@@ -634,8 +634,8 @@ static void test_install_printf(void) {
 
         assert_se(specifier_machine_id('m', NULL, NULL, &mid) >= 0 && mid);
         assert_se(specifier_boot_id('b', NULL, NULL, &bid) >= 0 && bid);
-        assert_se((host = gethostname_malloc()));
-        assert_se((user = uid_to_name(getuid())));
+        assert_se(host = gethostname_malloc());
+        assert_se(user = uid_to_name(getuid()));
         assert_se(asprintf(&uid, UID_FMT, getuid()) >= 0);
 
 #define expect(src, pattern, result)                                    \