From fd8279d0d48e423cd5d78b2466fbb46bd0119210 Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Mon, 22 Apr 2024 21:48:14 +0200 Subject: [PATCH 1/4] Fs: Invert logic to reduce indentation This refactor isn't very appealing alone, but it prepares the code for the following commits. Link: Cc: Andrew Clayton Cc: Liam Crilly Cc: Konstantin Pavlov Cc: Andy Postnikov Signed-off-by: Alejandro Colomar --- src/nxt_fs.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/nxt_fs.c b/src/nxt_fs.c index e10c5bcb2..0c8938707 100644 --- a/src/nxt_fs.c +++ b/src/nxt_fs.c @@ -62,11 +62,14 @@ nxt_fs_mkdir_parent(const u_char *path, mode_t mode) ret = NXT_OK; ptr = strrchr(dir, '/'); - if (nxt_fast_path(ptr != NULL)) { - *ptr = '\0'; - ret = nxt_fs_mkdir((const u_char *) dir, mode); + if (nxt_slow_path(ptr == NULL)) { + goto out_free; } + *ptr = '\0'; + ret = nxt_fs_mkdir((const u_char *) dir, mode); + +out_free: nxt_free(dir); return ret; From 1f20427e32b2d5564f0089f4ad7480ae51c1dda8 Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Mon, 22 Apr 2024 22:40:43 +0200 Subject: [PATCH 2/4] Fs: Correctly handle "/" in nxt_fs_mkdir_parent() The previous code attempted to mkdir(""), that is an empty string. Since "/" necessarily exists, just goto out_free. Link: Cc: Andrew Clayton Cc: Liam Crilly Cc: Konstantin Pavlov Cc: Andy Postnikov Signed-off-by: Alejandro Colomar --- src/nxt_fs.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/nxt_fs.c b/src/nxt_fs.c index 0c8938707..532ec57e2 100644 --- a/src/nxt_fs.c +++ b/src/nxt_fs.c @@ -1,5 +1,6 @@ /* * Copyright (C) NGINX, Inc. + * Copyright 2024, Alejandro Colomar */ #include @@ -62,7 +63,7 @@ nxt_fs_mkdir_parent(const u_char *path, mode_t mode) ret = NXT_OK; ptr = strrchr(dir, '/'); - if (nxt_slow_path(ptr == NULL)) { + if (nxt_slow_path(ptr == NULL || ptr == dir)) { goto out_free; } From 4ed9df7c6d003e80ceb692d4b7faece2057e1715 Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Mon, 22 Apr 2024 22:47:27 +0200 Subject: [PATCH 3/4] Fs: Make parents recursively for the pid file and the control socket Build systems should not attempt to create $runstatedir (or anything under it). Doing so causes warnings in packaging systems, such as in Alpine Linux, as reported by Andy. But unitd(8) can be configured to be installed under /opt, or other trees, where no directories exist before hand. Expecting that the user creates the entire directory trees that unit will need is a bit unreasonable. Instead, we can just create any directories that we need, with all their parents. Fixes: 57fc9201cb91 ("Socket: Created control socket & pid file directories.") Link: Reported-by: Andy Postnikov Cc: Andrew Clayton Cc: Liam Crilly Cc: Konstantin Pavlov Signed-off-by: Alejandro Colomar --- src/nxt_controller.c | 2 +- src/nxt_fs.c | 17 +++++++++++++++-- src/nxt_fs.h | 2 +- src/nxt_runtime.c | 2 +- 4 files changed, 18 insertions(+), 5 deletions(-) diff --git a/src/nxt_controller.c b/src/nxt_controller.c index eb814321d..f396daa97 100644 --- a/src/nxt_controller.c +++ b/src/nxt_controller.c @@ -695,7 +695,7 @@ nxt_runtime_controller_socket(nxt_task_t *task, nxt_runtime_t *rt) if (ls->sockaddr->u.sockaddr.sa_family == AF_UNIX) { const char *path = ls->sockaddr->u.sockaddr_un.sun_path; - nxt_fs_mkdir_parent((const u_char *) path, 0755); + nxt_fs_mkdir_parents_dirname((const u_char *) path, 0755); } #endif diff --git a/src/nxt_fs.c b/src/nxt_fs.c index 532ec57e2..649986f71 100644 --- a/src/nxt_fs.c +++ b/src/nxt_fs.c @@ -5,6 +5,8 @@ #include +#include + static nxt_int_t nxt_fs_mkdir(const u_char *dir, mode_t mode); @@ -50,7 +52,7 @@ nxt_fs_mkdir_all(const u_char *dir, mode_t mode) nxt_int_t -nxt_fs_mkdir_parent(const u_char *path, mode_t mode) +nxt_fs_mkdir_parents_dirname(const u_char *path, mode_t mode) { char *ptr, *dir; nxt_int_t ret; @@ -63,12 +65,23 @@ nxt_fs_mkdir_parent(const u_char *path, mode_t mode) ret = NXT_OK; ptr = strrchr(dir, '/'); - if (nxt_slow_path(ptr == NULL || ptr == dir)) { + if (ptr == dir || nxt_slow_path(ptr == NULL)) { goto out_free; } *ptr = '\0'; + if (strcmp(dir, (const char *) path) != 0) + { + ret = nxt_fs_mkdir_parents_dirname((const u_char *) dir, mode); + if (nxt_slow_path(ret == NXT_ERROR)) { + goto out_free; + } + } + ret = nxt_fs_mkdir((const u_char *) dir, mode); + if (ret == NXT_ERROR && errno == EEXIST) { + ret = NXT_OK; + } out_free: nxt_free(dir); diff --git a/src/nxt_fs.h b/src/nxt_fs.h index c8868d809..bc973e269 100644 --- a/src/nxt_fs.h +++ b/src/nxt_fs.h @@ -6,7 +6,7 @@ #define _NXT_FS_H_INCLUDED_ -nxt_int_t nxt_fs_mkdir_parent(const u_char *path, mode_t mode); +nxt_int_t nxt_fs_mkdir_parents_dirname(const u_char *path, mode_t mode); nxt_int_t nxt_fs_mkdir_all(const u_char *dir, mode_t mode); diff --git a/src/nxt_runtime.c b/src/nxt_runtime.c index 0e7f879e1..2c77e13c4 100644 --- a/src/nxt_runtime.c +++ b/src/nxt_runtime.c @@ -1490,7 +1490,7 @@ nxt_runtime_pid_file_create(nxt_task_t *task, nxt_file_name_t *pid_file) file.name = pid_file; - nxt_fs_mkdir_parent(pid_file, 0755); + nxt_fs_mkdir_parents_dirname(pid_file, 0755); n = nxt_file_open(task, &file, O_WRONLY, O_CREAT | O_TRUNC, NXT_FILE_DEFAULT_ACCESS); From bb26587a24f6390cbd5314bb89218b3bf83e8726 Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Mon, 22 Apr 2024 20:59:38 +0200 Subject: [PATCH 4/4] Auto: Don't install $runstatedir This directory should exist already in the system, and if not, it should (and will) be created at run time, not at install time. It triggered a warning in Alpine Linux's packaging system: ERROR: unit*: Packages must not put anything under /var/run Fixes: 5a37171f733f ("Added default values for pathnames.") Fixes: 57fc9201cb91 ("Socket: Created control socket & pid file directories.") Closes: Reported-by: Andy Postnikov Cc: Andrew Clayton Cc: Liam Crilly Cc: Konstantin Pavlov Signed-off-by: Alejandro Colomar --- auto/make | 2 -- configure | 1 - 2 files changed, 3 deletions(-) diff --git a/auto/make b/auto/make index 2788b9f52..515d69e98 100644 --- a/auto/make +++ b/auto/make @@ -434,8 +434,6 @@ ${NXT_DAEMON}-install: $NXT_DAEMON install-check || install -d \$(DESTDIR)$NXT_STATEDIR test -d \$(DESTDIR)$NXT_LOGDIR \ || install -d \$(DESTDIR)$NXT_LOGDIR - test -d \$(DESTDIR)$NXT_RUNSTATEDIR \ - || install -d \$(DESTDIR)$NXT_RUNSTATEDIR manpage-install: manpage install-check test -d \$(DESTDIR)$NXT_MANDIR/man8 \ diff --git a/configure b/configure index 2cb4d457e..42c861fb3 100755 --- a/configure +++ b/configure @@ -67,7 +67,6 @@ mkdir -p $NXT_BUILD_DIR/src mkdir -p $NXT_BUILD_DIR/src/test mkdir -p $NXT_BUILD_DIR/var/lib/unit mkdir -p $NXT_BUILD_DIR/var/log/unit -mkdir -p $NXT_BUILD_DIR/var/run/unit > $NXT_AUTOCONF_ERR