search for: nbd_internal_fork_safe_itoa

...al.h @@ -384,5 +384,7 @@ extern void nbd_internal_hexdump (const void *data, size_t len, FILE *fp); extern size_t nbd_internal_string_list_length (char **argv); extern char **nbd_internal_copy_string_list (char **argv); extern void nbd_internal_free_string_list (char **argv); +extern const char *nbd_internal_fork_safe_itoa (long v, char *buf, size_t len); +extern void nbd_internal_fork_safe_perror (const char *s); #endif /* LIBNBD_INTERNAL_H */ diff --git a/lib/utils.c b/lib/utils.c index 2d7fbf3..644781b 100644 --- a/lib/utils.c +++ b/lib/utils.c @@ -20,7 +20,10 @@ #include <stdio.h> #include <stdlib...

...s one. > + > +/* Like sprintf (s, "%ld", v). The caller must supply a scratch > + * buffer which is large enough for the result (32 bytes is fine), but > + * note that the returned string does not point to the start of this > + * buffer. > + */ > +const char * > +nbd_internal_fork_safe_itoa (long v, char *buf, size_t bufsize) > +{ > + size_t i = bufsize - 1; > + bool neg = false; > + > + buf[i--] = '\0'; > + if (v < 0) { > + neg = true; > + v = -v; /* XXX fails for INT_MIN */ Easy enough to work around. In the parameter...

...(environ[i], "LISTEN_FDS=", 11) != 0 && + strncmp (environ[i], "LISTEN_FDNAMES=", 15) != 0) { char *copy = strdup (environ[i]); if (copy == NULL) goto err; @@ -194,7 +199,7 @@ CONNECT_SA.START: char buf[32]; const char *v = nbd_internal_fork_safe_itoa ((long) getpid (), buf, sizeof buf); - strcpy (&env.ptr[0][PREFIX_LENGTH], v); + strcpy (&env.ptr[0][strlen ("LISTEN_FDS=")], v); /* Restore SIGPIPE back to SIG_DFL. */ signal (SIGPIPE, SIG_DFL); -- 2.39.0

v1 was posted here: https://www.redhat.com/archives/libguestfs/2019-September/thread.html#00337 v2: - Drop the first patch. - Hopefully fix the multiple issues with fork-safety and general behaviour on error paths. Note this requires execvpe for which there seems to be no equivalent on FreeBSD, except some kind of tedious path parsing (but can we assign to environ?) Rich.

...) != 0 && > + strncmp (environ[i], "LISTEN_FDNAMES=", 15) != 0) { > char *copy = strdup (environ[i]); > if (copy == NULL) > goto err; > @@ -194,7 +199,7 @@ CONNECT_SA.START: > char buf[32]; > const char *v = > nbd_internal_fork_safe_itoa ((long) getpid (), buf, sizeof buf); > - strcpy (&env.ptr[0][PREFIX_LENGTH], v); > + strcpy (&env.ptr[0][strlen ("LISTEN_FDS=")], v); > > /* Restore SIGPIPE back to SIG_DFL. */ > signal (SIGPIPE, SIG_DFL); I really didn't want to obsess about th...

...ry about, the solution would be pre-loading the entire message into a single buffer, then calling xwrite() just once - but that's far more effort for something we don't anticipate hitting in normal usage anyways. I'm happy if you ignore this whole paragraph of mine. > + line_out = nbd_internal_fork_safe_itoa (line, line_buf, sizeof line_buf); > + xwrite (STDERR_FILENO, line_out, strlen (line_out)); > + xwrite (STDERR_FILENO, ": ", 2); > + xwrite (STDERR_FILENO, func, strlen (func)); > + xwrite (STDERR_FILENO, ": Assertion `", 13); > + xwrite (STDERR_FILENO, asser...

...ncmp (environ[i], "LISTEN_FDNAMES=", 15) != 0) { > > char *copy = strdup (environ[i]); > > if (copy == NULL) > > goto err; > > @@ -194,7 +199,7 @@ CONNECT_SA.START: > > char buf[32]; > > const char *v = > > nbd_internal_fork_safe_itoa ((long) getpid (), buf, sizeof buf); > > - strcpy (&env.ptr[0][PREFIX_LENGTH], v); > > + strcpy (&env.ptr[0][strlen ("LISTEN_FDS=")], v); > > > > /* Restore SIGPIPE back to SIG_DFL. */ > > signal (SIGPIPE, SIG_DFL); > > I really...

This is version 4 of the following sub-series: [libnbd PATCH v3 06/29] lib/utils: introduce xwrite() as a more robust write() [libnbd PATCH v3 07/29] lib/utils: add async-signal-safe assert() [libnbd PATCH v3 08/29] lib/utils: add unit test for async-signal-safe assert() http://mid.mail-archive.com/20230215141158.2426855-7-lersek at redhat.com

...if (pid == 0) { /* child - run command */ if (s != FIRST_SOCKET_ACTIVATION_FD) { if (dup2 (s, FIRST_SOCKET_ACTIVATION_FD) == -1) { nbd_internal_fork_safe_perror ("dup2"); @@ -189,45 +196,47 @@ CONNECT_SA.START: char buf[32]; const char *v = nbd_internal_fork_safe_itoa ((long) getpid (), buf, sizeof buf); strcpy (&env.ptr[0][PREFIX_LENGTH], v); /* Restore SIGPIPE back to SIG_DFL. */ if (signal (SIGPIPE, SIG_DFL) == SIG_ERR) { nbd_internal_fork_safe_perror ("signal"); _exit (126); } - environ = env.ptr; - e...

...more effort for > something we don't anticipate hitting in normal usage anyways. I'm > happy if you ignore this whole paragraph of mine. Any single buffer presents the problem of sizing the buffer appropriately, which we can't do in this context :) > >> + line_out = nbd_internal_fork_safe_itoa (line, line_buf, sizeof line_buf); >> + xwrite (STDERR_FILENO, line_out, strlen (line_out)); >> + xwrite (STDERR_FILENO, ": ", 2); >> + xwrite (STDERR_FILENO, func, strlen (func)); >> + xwrite (STDERR_FILENO, ": Assertion `", 13); >> + xwrite (...

V3 was here: <http://mid.mail-archive.com/20230215141158.2426855-1-lersek at redhat.com>. See the Notes section on each patch for the v4 updates. The series is nearly ready for merging: every patch has at least one R-b tag, except "socket activation: avoid manipulating the sign bit". The series builds, and passes "make check" and "make check-valgrind", at

...6); > + } > + if (fcntl (s, F_SETFD, flags & ~FD_CLOEXEC) == -1) { > + nbd_internal_fork_safe_perror ("fcntl: F_SETFD"); > + _exit (126); > + } > + } > + Looks correct. > + char buf[32]; > + const char *v = > + nbd_internal_fork_safe_itoa ((long) getpid (), buf, sizeof buf); > + strcpy (&env[0][11], v); We're using the magic '11' in several places, maybe it's worth a #define to make it obvious it is strlen("LISTEN_PID=") ? > + > + /* Restore SIGPIPE back to SIG_DFL. */ > + signal...

This is an alternative approach to https://listman.redhat.com/archives/libguestfs/2023-January/030535.html After discussing this with Dan Berrange we came to the conclusion that you really might want to set LISTEN_FDNAMES to arbitrary short strings (or not set it). Especially when talking to qemu-storage-daemon which would allow you to use these names on the command line. Rich.

...l_fork_safe_perror ("fcntl: F_GETFD"); + _exit (126); + } + if (fcntl (s, F_SETFD, flags & ~FD_CLOEXEC) == -1) { + nbd_internal_fork_safe_perror ("fcntl: F_SETFD"); + _exit (126); + } + } + + char buf[32]; + const char *v = + nbd_internal_fork_safe_itoa ((long) getpid (), buf, sizeof buf); + strcpy (&env[0][11], v); + + /* Restore SIGPIPE back to SIG_DFL. */ + signal (SIGPIPE, SIG_DFL); + + execvpe (h->argv[0], h->argv, env); + nbd_internal_fork_safe_perror (h->argv[0]); + if (errno == ENOENT) + _exit (127); +...

V4 was here (incorrectly versioned on the mailing list as v3): <http://mid.mail-archive.com/20230323121016.1442655-1-lersek at redhat.com>. See the Notes section on each patch for the v5 updates. Laszlo Ersek (2): socket activation: generalize environment construction socket activation: set LISTEN_FDNAMES Richard W.M. Jones (2): common/include: Copy ascii-ctype functions from nbdkit

This adds coalescing of adjacent extents of the same type, as mentioned by Eric Blake in the commit message here: https://github.com/libguestfs/libnbd/commit/46072f6611f80245846a445766da071e457b00cd The patch series is rather long because it detours through adding the <vector.h> library from nbdkit into libnbd and replacing ad hoc uses of realloc, char ** etc in various places. Rich.