Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mkdir -p $runstatedir at run time, not install time (round 2) #1235

Merged
merged 11 commits into from
Jun 18, 2024
Merged

mkdir -p $runstatedir at run time, not install time (round 2) #1235

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
3db000a
fs: Rename nxt_fs_mkdir_parent() => nxt_fs_mkdir_dirname()
alejandro-colomar Apr 24, 2024
40eaf4e
fs: Rename nxt_fs_mkdir_all() => nxt_fs_mkdir_p()
alejandro-colomar Apr 24, 2024
af6a67f
fs: Use branchless code in nxt_fs_mkdir_p()
alejandro-colomar Apr 23, 2024
2eb2f60
fs: Use a temporary variable in nxt_fs_mkdir_p()
alejandro-colomar Apr 23, 2024
c6ce038
fs: Accept relative paths in nxt_fs_mkdir_p()
alejandro-colomar Apr 23, 2024
3873f98
fs: Accept path names of length 1 in nxt_fs_mkdir_p()
alejandro-colomar Apr 23, 2024
7b7b303
fs: Invert logic to reduce indentation in nxt_fs_mkdir_dirname()
alejandro-colomar Apr 22, 2024
a9aed20
fs: Correctly handle "/" in nxt_fs_mkdir_dirname()
alejandro-colomar Apr 22, 2024
eca3834
fs: Make the full directory path for the pid file and the control socket
alejandro-colomar Apr 22, 2024
2e2e5d1
auto: Don't install $runstatedir
alejandro-colomar Apr 22, 2024
d96d583
Use octal instead of mode macros
alejandro-colomar Apr 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions auto/make
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down
7 changes: 3 additions & 4 deletions auto/shmem
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@ nxt_feature_test="#include <sys/mman.h>

shm_unlink(name);

int fd = shm_open(name, O_CREAT | O_EXCL | O_RDWR,
S_IRUSR | S_IWUSR);
int fd = shm_open(name, O_CREAT | O_EXCL | O_RDWR, 0600);
if (fd == -1)
return 1;

Expand Down Expand Up @@ -68,7 +67,7 @@ if [ $nxt_found = no ]; then
shm_unlink(name);

int fd = shm_open(name, O_CREAT | O_EXCL | O_RDWR,
S_IRUSR | S_IWUSR);
0600);
if (fd == -1)
return 1;

Expand All @@ -95,7 +94,7 @@ nxt_feature_test="#include <sys/mman.h>
#include <sys/stat.h>

int main(void) {
int fd = shm_open(SHM_ANON, O_RDWR, S_IRUSR | S_IWUSR);
int fd = shm_open(SHM_ANON, O_RDWR, 0600);
if (fd == -1)
return 1;

Expand Down
1 change: 0 additions & 1 deletion configure
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,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
Expand Down
2 changes: 1 addition & 1 deletion src/nxt_cgroup.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ nxt_cgroup_proc_add(nxt_task_t *task, nxt_process_t *process)
return NXT_ERROR;
}

ret = nxt_fs_mkdir_all((const u_char *) cgprocs, 0777);
ret = nxt_fs_mkdir_p((const u_char *) cgprocs, 0777);
if (nxt_slow_path(ret == NXT_ERROR)) {
return NXT_ERROR;
}
Expand Down
2 changes: 1 addition & 1 deletion src/nxt_controller.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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_p_dirname((const u_char *) path, 0755);
}
#endif

Expand Down
34 changes: 17 additions & 17 deletions src/nxt_fs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/*
* Copyright (C) NGINX, Inc.
* Copyright 2024, Alejandro Colomar <alx@kernel.org>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heh, I'm not sure the below change constitutes a significant contribution...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess I could move it to the next commit, but that one was already better than the previous ones, which are trivial refactors.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since that one already accumulates several changes to that page, I guess it's fair.

Copy link
Contributor Author

@alejandro-colomar alejandro-colomar Apr 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Although, thanks to all the refactors, the next commit is also just 2 lines in that file, and one is a rename :D)

*/

#include <nxt_main.h>
Expand All @@ -9,35 +10,31 @@ static nxt_int_t nxt_fs_mkdir(const u_char *dir, mode_t mode);


nxt_int_t
nxt_fs_mkdir_all(const u_char *dir, mode_t mode)
nxt_fs_mkdir_p(const u_char *dir, mode_t mode)
{
char *start, *end, *dst;
size_t dirlen;
char path[PATH_MAX];
char *start, *end, *dst;
size_t dirlen;
nxt_int_t ret;
char path[PATH_MAX];
Comment on lines -14 to +18
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I'm not fussed about having Christmas tree variable type ordering and it was 'wrong' before, so meh...

( I could also quite happily live without the alignment, and the multiple variable declaration on a single line for that matter, it would make changes like this simpler)


dirlen = nxt_strlen(dir);

nxt_assert(dirlen < PATH_MAX && dirlen > 1 && dir[0] == '/');
nxt_assert(dirlen < PATH_MAX && dirlen > 0);

dst = path;
start = (char *) dir;

while (*start != '\0') {
if (*start == '/') {
*dst++ = *start++;
}

end = strchr(start, '/');
end = strchr(start + 1, '/');
if (end == NULL) {
end = ((char *)dir + dirlen);
}

dst = nxt_cpymem(dst, start, end - start);
*dst = '\0';

if (nxt_slow_path(nxt_fs_mkdir((u_char *) path, mode) != NXT_OK
&& nxt_errno != EEXIST))
{
ret = nxt_fs_mkdir((u_char *) path, mode);
if (nxt_slow_path(ret != NXT_OK && nxt_errno != EEXIST)) {
return NXT_ERROR;
}

Expand All @@ -49,7 +46,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_p_dirname(const u_char *path, mode_t mode)
{
char *ptr, *dir;
nxt_int_t ret;
Expand All @@ -62,11 +59,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 (ptr == dir || nxt_slow_path(ptr == NULL)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if path == "/" should constitute an error?, as that's clearly not what you'd want.

Copy link
Contributor Author

@alejandro-colomar alejandro-colomar Apr 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One may legitimatelly want to --control=/unit.control.sock, if they don't care about FHS and having an organized system.

We probably agree that they should be punished by such an offense, but misbehaving, or even prohibiting, is probably not a fair punishment.

goto out_free;
}

*ptr = '\0';
ret = nxt_fs_mkdir_p((const u_char *) dir, mode);

out_free:
nxt_free(dir);

return ret;
Expand Down
4 changes: 2 additions & 2 deletions src/nxt_fs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
#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_all(const u_char *dir, mode_t mode);
nxt_int_t nxt_fs_mkdir_p_dirname(const u_char *path, mode_t mode);
nxt_int_t nxt_fs_mkdir_p(const u_char *dir, mode_t mode);


#endif /* _NXT_FS_H_INCLUDED_ */
2 changes: 1 addition & 1 deletion src/nxt_isolation.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -780,7 +780,7 @@ nxt_isolation_prepare_rootfs(nxt_task_t *task, nxt_process_t *process)
continue;
}

ret = nxt_fs_mkdir_all(dst, S_IRWXU | S_IRWXG | S_IRWXO);
ret = nxt_fs_mkdir_p(dst, 0777);
if (nxt_slow_path(ret != NXT_OK)) {
nxt_alert(task, "mkdir(%s) %E", dst, nxt_errno);
goto undo;
Expand Down
2 changes: 1 addition & 1 deletion src/nxt_listen_socket.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ nxt_listen_socket_create(nxt_task_t *task, nxt_mp_t *mp,
nxt_runtime_t *rt = thr->runtime;

name = (nxt_file_name_t *) sa->u.sockaddr_un.sun_path;
access = rt->control_mode > 0 ? rt->control_mode : S_IRUSR | S_IWUSR;
access = rt->control_mode > 0 ? rt->control_mode : 0600;

if (nxt_file_set_access(name, access) != NXT_OK) {
goto listen_fail;
Expand Down
4 changes: 1 addition & 3 deletions src/nxt_main_process.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1275,13 +1275,11 @@ nxt_main_listening_socket(nxt_sockaddr_t *sa, nxt_listening_socket_t *ls)
&& sa->u.sockaddr_un.sun_path[0] != '\0')
{
char *filename;
mode_t access;
nxt_thread_t *thr;

filename = sa->u.sockaddr_un.sun_path;
access = (S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH);

if (chmod(filename, access) != 0) {
if (chmod(filename, 0666) != 0) {
ls->end = nxt_sprintf(ls->start, ls->end,
"chmod(\\\"%s\\\") failed %E",
filename, nxt_errno);
Expand Down
6 changes: 2 additions & 4 deletions src/nxt_port_memory.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -393,8 +393,7 @@ nxt_shm_open(nxt_task_t *task, size_t size)

#elif (NXT_HAVE_SHM_OPEN_ANON)

fd = shm_open(SHM_ANON, O_RDWR, S_IRUSR | S_IWUSR);

fd = shm_open(SHM_ANON, O_RDWR, 0600);
if (nxt_slow_path(fd == -1)) {
nxt_alert(task, "shm_open(SHM_ANON) failed %E", nxt_errno);

Expand All @@ -408,8 +407,7 @@ nxt_shm_open(nxt_task_t *task, size_t size)
/* Just in case. */
shm_unlink((char *) name);

fd = shm_open((char *) name, O_CREAT | O_EXCL | O_RDWR, S_IRUSR | S_IWUSR);

fd = shm_open((char *) name, O_CREAT | O_EXCL | O_RDWR, 0600);
if (nxt_slow_path(fd == -1)) {
nxt_alert(task, "shm_open(%s) failed %E", name, nxt_errno);

Expand Down
8 changes: 3 additions & 5 deletions src/nxt_runtime.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -895,8 +895,7 @@ nxt_runtime_conf_init(nxt_task_t *task, nxt_runtime_t *rt)
return NXT_ERROR;
}

ret = mkdir((char *) file_name.start, S_IRWXU);

ret = mkdir((char *) file_name.start, 0700);
if (nxt_fast_path(ret == 0 || nxt_errno == EEXIST)) {
rt->certs.length = file_name.len;
rt->certs.start = file_name.start;
Expand All @@ -912,8 +911,7 @@ nxt_runtime_conf_init(nxt_task_t *task, nxt_runtime_t *rt)
return NXT_ERROR;
}

ret = mkdir((char *) file_name.start, S_IRWXU);

ret = mkdir((char *) file_name.start, 0700);
if (nxt_fast_path(ret == 0 || nxt_errno == EEXIST)) {
rt->scripts.length = file_name.len;
rt->scripts.start = file_name.start;
Expand Down Expand Up @@ -1490,7 +1488,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_p_dirname(pid_file, 0755);

n = nxt_file_open(task, &file, O_WRONLY, O_CREAT | O_TRUNC,
NXT_FILE_DEFAULT_ACCESS);
Expand Down
4 changes: 2 additions & 2 deletions src/nxt_unit.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3857,7 +3857,7 @@ nxt_unit_shm_open(nxt_unit_ctx_t *ctx, size_t size)

#elif (NXT_HAVE_SHM_OPEN_ANON)

fd = shm_open(SHM_ANON, O_RDWR, S_IRUSR | S_IWUSR);
fd = shm_open(SHM_ANON, O_RDWR, 0600);
if (nxt_slow_path(fd == -1)) {
nxt_unit_alert(ctx, "shm_open(SHM_ANON) failed: %s (%d)",
strerror(errno), errno);
Expand All @@ -3870,7 +3870,7 @@ nxt_unit_shm_open(nxt_unit_ctx_t *ctx, size_t size)
/* Just in case. */
shm_unlink(name);

fd = shm_open(name, O_CREAT | O_EXCL | O_RDWR, S_IRUSR | S_IWUSR);
fd = shm_open(name, O_CREAT | O_EXCL | O_RDWR, 0600);
if (nxt_slow_path(fd == -1)) {
nxt_unit_alert(ctx, "shm_open(%s) failed: %s (%d)", name,
strerror(errno), errno);
Expand Down
Loading