core: simplify how we create socket fds

Let's always return the allocated fds as return values where possible,
and make more use of _cleanup_close_

1 files changed, 94 insertions, 118 deletions

diff --git a/src/core/socket.c b/src/core/socket.c
index 2d927f580a..c0c11e4f6a 100644
--- a/src/core/socket.c
+++ b/src/core/socket.c
@@ -19,38 +19,39 @@
   along with systemd; If not, see <http://www.gnu.org/licenses/>.
 ***/
 
-#include <sys/stat.h>
-#include <unistd.h>
+#include <arpa/inet.h>
 #include <errno.h>
 #include <fcntl.h>
-#include <sys/epoll.h>
-#include <signal.h>
-#include <arpa/inet.h>
-#include <netinet/tcp.h>
 #include <mqueue.h>
+#include <netinet/tcp.h>
+#include <signal.h>
+#include <sys/epoll.h>
+#include <sys/stat.h>
+#include <unistd.h>
 
 #include "sd-event.h"
+
+#include "bus-error.h"
+#include "bus-util.h"
+#include "copy.h"
+#include "dbus-socket.h"
+#include "def.h"
+#include "exit-status.h"
+#include "formats-util.h"
+#include "label.h"
 #include "log.h"
-#include "strv.h"
+#include "missing.h"
 #include "mkdir.h"
 #include "path-util.h"
-#include "unit-name.h"
-#include "unit-printf.h"
-#include "missing.h"
-#include "special.h"
-#include "label.h"
-#include "exit-status.h"
-#include "def.h"
-#include "smack-util.h"
-#include "bus-util.h"
-#include "bus-error.h"
 #include "selinux-util.h"
-#include "dbus-socket.h"
-#include "unit.h"
-#include "formats-util.h"
 #include "signal-util.h"
+#include "smack-util.h"
 #include "socket.h"
-#include "copy.h"
+#include "special.h"
+#include "strv.h"
+#include "unit-name.h"
+#include "unit-printf.h"
+#include "unit.h"
 
 static const UnitActiveState state_translation_table[_SOCKET_STATE_MAX] = {
         [SOCKET_DEAD] = UNIT_INACTIVE,
@@ -967,38 +968,36 @@ static void socket_apply_fifo_options(Socket *s, int fd) {
 static int fifo_address_create(
                 const char *path,
                 mode_t directory_mode,
-                mode_t socket_mode,
-                int *_fd) {
+                mode_t socket_mode) {
 
-        int fd = -1, r = 0;
-        struct stat st;
+        _cleanup_close_ int fd = -1;
         mode_t old_mask;
+        struct stat st;
+        int r;
 
         assert(path);
-        assert(_fd);
 
         mkdir_parents_label(path, directory_mode);
 
         r = mac_selinux_create_file_prepare(path, S_IFIFO);
         if (r < 0)
-                goto fail;
+                return r;
 
         /* Enforce the right access mode for the fifo */
         old_mask = umask(~ socket_mode);
 
         /* Include the original umask in our mask */
-        umask(~socket_mode | old_mask);
+        (void) umask(~socket_mode | old_mask);
 
         r = mkfifo(path, socket_mode);
-        umask(old_mask);
+        (void) umask(old_mask);
 
         if (r < 0 && errno != EEXIST) {
                 r = -errno;
                 goto fail;
         }
 
-        fd = open(path,
-                  O_RDWR | O_CLOEXEC | O_NOCTTY | O_NONBLOCK | O_NOFOLLOW);
+        fd = open(path, O_RDWR | O_CLOEXEC | O_NOCTTY | O_NONBLOCK | O_NOFOLLOW);
         if (fd < 0) {
                 r = -errno;
                 goto fail;
@@ -1015,13 +1014,14 @@ static int fifo_address_create(
             (st.st_mode & 0777) != (socket_mode & ~old_mask) ||
             st.st_uid != getuid() ||
             st.st_gid != getgid()) {
-
                 r = -EEXIST;
                 goto fail;
         }
 
-        *_fd = fd;
-        return 0;
+        r = fd;
+        fd = -1;
+
+        return r;
 
 fail:
         mac_selinux_create_file_clear();
@@ -1030,51 +1030,36 @@ fail:
         return r;
 }
 
-static int special_address_create(
-                const char *path,
-                int *_fd) {
-
-        int fd = -1, r = 0;
+static int special_address_create(const char *path) {
+        _cleanup_close_ int fd = -1;
         struct stat st;
+        int r;
 
         assert(path);
-        assert(_fd);
 
         fd = open(path, O_RDONLY|O_CLOEXEC|O_NOCTTY|O_NONBLOCK|O_NOFOLLOW);
-        if (fd < 0) {
-                r = -errno;
-                goto fail;
-        }
+        if (fd < 0)
+                return -errno;
 
-        if (fstat(fd, &st) < 0) {
-                r = -errno;
-                goto fail;
-        }
+        if (fstat(fd, &st) < 0)
+                return -errno;
 
         /* Check whether this is a /proc, /sys or /dev file or char device */
-        if (!S_ISREG(st.st_mode) && !S_ISCHR(st.st_mode)) {
-                r = -EEXIST;
-                goto fail;
-        }
-
-        *_fd = fd;
-        return 0;
+        if (!S_ISREG(st.st_mode) && !S_ISCHR(st.st_mode))
+                return -EEXIST;
 
-fail:
-        safe_close(fd);
+        r = fd;
+        fd = -1;
 
         return r;
 }
 
-static int ffs_address_create(
-                const char *path,
-                int *_fd) {
-
+static int ffs_address_create(const char *path) {
         _cleanup_close_ int fd = -1;
         struct stat st;
+        int r;
 
         assert(path);
-        assert(_fd);
 
         fd = open(path, O_RDWR|O_CLOEXEC|O_NOCTTY|O_NONBLOCK|O_NOFOLLOW);
         if (fd < 0)
@@ -1087,32 +1072,32 @@ static int ffs_address_create(
         if (!S_ISREG(st.st_mode))
                 return -EEXIST;
 
-        *_fd = fd;
+        r = fd;
         fd = -1;
 
-        return 0;
+        return r;
 }
 
 static int mq_address_create(
                 const char *path,
                 mode_t mq_mode,
                 long maxmsg,
-                long msgsize,
-                int *_fd) {
+                long msgsize) {
 
-        int fd = -1, r = 0;
+        _cleanup_close_ int fd = -1;
         struct stat st;
         mode_t old_mask;
         struct mq_attr _attr, *attr = NULL;
+        int r;
 
         assert(path);
-        assert(_fd);
 
         if (maxmsg > 0 && msgsize > 0) {
-                zero(_attr);
-                _attr.mq_flags = O_NONBLOCK;
-                _attr.mq_maxmsg = maxmsg;
-                _attr.mq_msgsize = msgsize;
+                _attr = (struct mq_attr) {
+                        .mq_flags = O_NONBLOCK,
+                        .mq_maxmsg = maxmsg,
+                        .mq_msgsize = msgsize,
+                };
                 attr = &_attr;
         }
 
@@ -1120,33 +1105,24 @@ static int mq_address_create(
         old_mask = umask(~ mq_mode);
 
         /* Include the original umask in our mask */
-        umask(~mq_mode | old_mask);
+        (void) umask(~mq_mode | old_mask);
         fd = mq_open(path, O_RDONLY|O_CLOEXEC|O_NONBLOCK|O_CREAT, mq_mode, attr);
-        umask(old_mask);
+        (void) umask(old_mask);
 
-        if (fd < 0) {
-                r = -errno;
-                goto fail;
-        }
+        if (fd < 0)
+                return -errno;
 
-        if (fstat(fd, &st) < 0) {
-                r = -errno;
-                goto fail;
-        }
+        if (fstat(fd, &st) < 0)
+                return -errno;
 
         if ((st.st_mode & 0777) != (mq_mode & ~old_mask) ||
             st.st_uid != getuid() ||
-            st.st_gid != getgid()) {
-
-                r = -EEXIST;
-                goto fail;
-        }
+            st.st_gid != getgid())
+                return -EEXIST;
 
-        *_fd = fd;
-        return 0;
+        r = fd;
+        fd = -1;
 
-fail:
-        safe_close(fd);
         return r;
 }
 
@@ -1166,8 +1142,7 @@ static int socket_symlink(Socket *s) {
         return 0;
 }
 
-static int ffs_write_descs(int fd, Unit *u) {
-        Service *s = SERVICE(u);
+static int ffs_write_descs(int fd, Service *s) {
         int r;
 
         if (!s->usb_function_descriptors || !s->usb_function_strings)
@@ -1175,11 +1150,9 @@ static int ffs_write_descs(int fd, Unit *u) {
 
         r = copy_file_fd(s->usb_function_descriptors, fd, false);
         if (r < 0)
-                return 0;
-
-        r = copy_file_fd(s->usb_function_strings, fd, false);
+                return r;
 
-        return r;
+        return copy_file_fd(s->usb_function_strings, fd, false);
 }
 
 static int select_ep(const struct dirent *d) {
@@ -1216,10 +1189,12 @@ static int ffs_dispatch_eps(SocketPort *p) {
 
                 path_kill_slashes(ep);
 
-                r = ffs_address_create(ep, &p->auxiliary_fds[k]);
+                r = ffs_address_create(ep);
                 if (r < 0)
                         goto fail;
 
+                p->auxiliary_fds[k] = r;
+
                 ++k;
                 free(ent[i]);
         }
@@ -1227,9 +1202,7 @@ static int ffs_dispatch_eps(SocketPort *p) {
         return r;
 
 fail:
-        while (k)
-                safe_close(p->auxiliary_fds[--k]);
-
+        close_many(p->auxiliary_fds, k);
         p->auxiliary_fds = mfree(p->auxiliary_fds);
         p->n_auxiliary_fds = 0;
 
@@ -1303,44 +1276,47 @@ static int socket_open_fds(Socket *s) {
 
                 } else  if (p->type == SOCKET_SPECIAL) {
 
-                        r = special_address_create(
-                                        p->path,
-                                        &p->fd);
-                        if (r < 0)
+                        p->fd = special_address_create(p->path);
+                        if (p->fd < 0) {
+                                r = p->fd;
                                 goto rollback;
+                        }
 
                 } else  if (p->type == SOCKET_FIFO) {
 
-                        r = fifo_address_create(
+                        p->fd = fifo_address_create(
                                         p->path,
                                         s->directory_mode,
-                                        s->socket_mode,
-                                        &p->fd);
-                        if (r < 0)
+                                        s->socket_mode);
+                        if (p->fd < 0) {
+                                r = p->fd;
                                 goto rollback;
+                        }
 
                         socket_apply_fifo_options(s, p->fd);
                         socket_symlink(s);
 
                 } else if (p->type == SOCKET_MQUEUE) {
 
-                        r = mq_address_create(
+                        p->fd = mq_address_create(
                                         p->path,
                                         s->socket_mode,
                                         s->mq_maxmsg,
-                                        s->mq_msgsize,
-                                        &p->fd);
-                        if (r < 0)
+                                        s->mq_msgsize);
+                        if (p->fd < 0) {
+                                r = p->fd;
                                 goto rollback;
+                        }
+
                 } else  if (p->type == SOCKET_USB_FUNCTION) {
 
-                        r = ffs_address_create(
-                                        p->path,
-                                        &p->fd);
-                        if (r < 0)
+                        p->fd = ffs_address_create(p->path);
+                        if (p->fd < 0) {
+                                r = p->fd;
                                 goto rollback;
+                        }
 
-                        r = ffs_write_descs(p->fd, s->service.unit);
+                        r = ffs_write_descs(p->fd, SERVICE(UNIT_DEREF(s->service)));
                         if (r < 0)
                                 goto rollback;