On Wed, Apr 05, 2023 at 01:58:00PM +0200, Stefano Brivio wrote:On Tue, 4 Apr 2023 11:46:28 +1000 David Gibson <david(a)gibson.dropbear.id.au> wrote:Sure, but as you say unrelated.This will make it easier to differentiate the options to those commands further in future. Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- test/nstool.c | 102 +++++++++++++++++++++++++++++++++----------------- 1 file changed, 68 insertions(+), 34 deletions(-) diff --git a/test/nstool.c b/test/nstool.c index 7e069b6..9ea7eeb 100644 --- a/test/nstool.c +++ b/test/nstool.c @@ -11,6 +11,7 @@ #include <stdio.h> #include <stdlib.h> #include <string.h> +#include <stdbool.h> #include <errno.h> #include <unistd.h> #include <sys/socket.h> @@ -37,19 +38,55 @@ static void usage(void) " terminate.\n"); } -static void hold(int fd, const struct sockaddr_un *addr) +static int connect_ctl(const char * sockpath, bool wait) { + int fd = socket(AF_UNIX, SOCK_STREAM, PF_UNIX); + struct sockaddr_un addr = { + .sun_family = AF_UNIX, + }; int rc; - rc = bind(fd, (struct sockaddr *)addr, sizeof(*addr)); + if (fd < 0) + die("socket(): %s\n", strerror(errno));Unrelated: it would be nice if die() added newlines eventually.Yeah, I guess so. That's not new, it was already like that in "nsholder pid", so I think something to fix separately.+ + strncpy(addr.sun_path, sockpath, UNIX_PATH_MAX); + + do { + rc = connect(fd, (struct sockaddr *)&addr, sizeof(addr)); + if (rc < 0 && + (!wait || (errno != ENOENT && errno != ECONNREFUSED))) + die("connect() to %s: %s\n", sockpath, strerror(errno));A (1ms?) delay would be nice to have here -- it's almost a busyloop, connect() fails fast.So, this is a consequence of the fact that the holder doesn't move into the background itself - it just sits in the foreground until terminated. That means that the typical usecase puts it into the background from the shell with &, which in turn means that when we reach the next shell command the socket may not be ready - or not even created. One of the things I had in mind for a hypothetical "nstool unshare" would be to avoid this and have it background itself once the socket is ready.+ } while (rc < 0); + + return fd; +} + +static void cmd_hold(int argc, char *argv[]) +{ + int fd = socket(AF_UNIX, SOCK_STREAM, PF_UNIX); + struct sockaddr_un addr = { + .sun_family = AF_UNIX, + }; + const char *sockpath = argv[1]; + int rc; + + if (argc != 2) + usage(); + + if (fd < 0) + die("socket(): %s\n", strerror(errno)); + + strncpy(addr.sun_path, sockpath, UNIX_PATH_MAX); + + rc = bind(fd, (struct sockaddr *)&addr, sizeof(addr)); if (rc < 0) - die("bind(): %s\n", strerror(errno)); + die("bind() to %s: %s\n", sockpath, strerror(errno)); rc = listen(fd, 0); if (rc < 0) - die("listen(): %s\n", strerror(errno)); + die("listen() on %s: %s\n", sockpath, strerror(errno)); - printf("nstool: local PID=%d local UID=%u local GID=%u\n", + printf("nstool hold: local PID=%d local UID=%u local GID=%u\n", getpid(), getuid(), getgid()); do { int afd = accept(fd, NULL, NULL); @@ -63,71 +100,68 @@ static void hold(int fd, const struct sockaddr_un *addr) die("read(): %s\n", strerror(errno)); } while (rc == 0); - unlink(addr->sun_path); + unlink(sockpath); } -static void pid(int fd, const struct sockaddr_un *addr) +static void cmd_pid(int argc, char *argv[]) { - int rc; + const char *sockpath = argv[1]; struct ucred peercred; socklen_t optlen = sizeof(peercred); + int fd, rc; - do { - rc = connect(fd, (struct sockaddr *)addr, sizeof(*addr)); - if (rc < 0 && errno != ENOENT && errno != ECONNREFUSED) - die("connect(): %s\n", strerror(errno)); - } while (rc < 0); + if (argc != 2) + usage(); + + fd = connect_ctl(sockpath, true);I didn't spot this earlier, but... does it really make sense to wait in cmd_pid(), also on ENOENT, rather than making 'hold' return only once the socket is ready?I don't think it would be outrageous to have 'nstool pid' failing if the holding process doesn't exist. Admittely, I'm biased by the few hundreds of times I needed to 'killall -9 nsholder' in the past months. :)So... I agree that's irritating, I've done it a similar number of times. However, I don't think that's really related to the question above - in my experience it's always been the holder process that's hung around, not something waiting on a holder.Uh.. I don't see where a compound literal would even go here. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibsonrc = getsockopt(fd, SOL_SOCKET, SO_PEERCRED, &peercred, &optlen); if (rc < 0) - die("getsockopet(SO_PEERCRED): %s\n", strerror(errno)); + die("getsockopet(SO_PEERCRED) %s: %s\n", + sockpath, strerror(errno)); close(fd); printf("%d\n", peercred.pid); } -static void stop(int fd, const struct sockaddr_un *addr) +static void cmd_stop(int argc, char *argv[]) { - int rc; + const char *sockpath = argv[1]; + int fd, rc; char buf = 'Q'; - rc = connect(fd, (struct sockaddr *)addr, sizeof(*addr)); - if (rc < 0) - die("connect(): %s\n", strerror(errno)); + if (argc != 2) + usage(); + + fd = connect_ctl(sockpath, false); rc = write(fd, &buf, sizeof(buf));Unrelated: a compound literal would make this more readable.