Re: [hackers] [slock] Use argv0 instead of passing "slock:" to die every time || FRIGN
On 14 February 2016 at 02:14, wrote: > commit b02c4d452a7942d4be3c69e6f98dafd35a2e4e78 > Author: FRIGN > AuthorDate: Sun Feb 14 02:13:54 2016 +0100 > Commit: FRIGN > CommitDate: Sun Feb 14 02:13:54 2016 +0100 > > Use argv0 instead of passing "slock:" to die every time > > diff --git a/slock.c b/slock.c > index 4531f95..a0ffed0 100644 > --- a/slock.c > +++ b/slock.c > @@ -46,6 +46,7 @@ static Bool failure = False; > static Bool rr; > static int rrevbase; > static int rrerrbase; > +static char *argv0; > > static void > die(const char *errstr, ...) > @@ -53,6 +54,7 @@ die(const char *errstr, ...) > va_list ap; > > va_start(ap, errstr); > + fprintf(stderr, "%s: ", argv0); I don't like this approach. Code should always output the original name of the program, not the name of the process. When someone renames slock into foo, then I cannot assert anymore what it really is all about, if I query ./foo -v or ./foo -h I'm for reverting to the original approach here -- btw. same for all other suckless tools. BR, Anselm
Re: [hackers] [slock] Use argv0 instead of passing "slock:" to die every time || FRIGN
On Mon, Feb 15, 2016 at 11:37:15AM +0100, FRIGN wrote: > On Mon, 15 Feb 2016 10:34:15 + > Dimitris Papastamos wrote: > > Hey Dimitris, > > > Yes this pattern is used in sbase. There is no point however > > replicating it to other projects. > > so how would you approach this? If slock was only a main()-function, > one could've thought to just pass argv[0] to each call of die. > But it is not. > > Would your approach be to just set argv0 as argv[0] and carry on > without modifying argc, argv? > I was surprised this idiomatic one-liner has received such opposition. The code was fine as it was.
Re: [hackers] [slock] Use argv0 instead of passing "slock:" to die every time || FRIGN
FRIGN wrote: > we came up with this in sbase/ubase as an idiomatic way to set argv0 > for tools that don't use arg.h. > We need argv0 here because I moved the prepending of the argv[0] into > die(), and thus we need to store the argv0 in a global variable. Heyho frign, sure, but I asked why you would *shift* argc and argv right at the start of main. If someone reads just the fork exec part and assumes argv[0] is `slock`, this is confusing and forcing the reader to search for the place where argv is changed. --Markus
Re: [hackers] [slock] Use argv0 instead of passing "slock:" to die every time || FRIGN
On Mon, 15 Feb 2016 10:34:15 + Dimitris Papastamos wrote: Hey Dimitris, > Yes this pattern is used in sbase. There is no point however > replicating it to other projects. so how would you approach this? If slock was only a main()-function, one could've thought to just pass argv[0] to each call of die. But it is not. Would your approach be to just set argv0 as argv[0] and carry on without modifying argc, argv? I was surprised this idiomatic one-liner has received such opposition. Cheers FRIGN -- FRIGN
Re: [hackers] [slock] Use argv0 instead of passing "slock:" to die every time || FRIGN
On Mon, Feb 15, 2016 at 11:30:59AM +0100, FRIGN wrote: > On Mon, 15 Feb 2016 11:22:08 +0100 > Markus Teich wrote: > > Hey Markus, > > > why this `argc--, argv++` shenanigans? I think it's more confusing rather > > than > > helping. > > we came up with this in sbase/ubase as an idiomatic way to set argv0 > for tools that don't use arg.h. > We need argv0 here because I moved the prepending of the argv[0] into > die(), and thus we need to store the argv0 in a global variable. Yes this pattern is used in sbase. There is no point however replicating it to other projects.
Re: [hackers] [slock] Use argv0 instead of passing "slock:" to die every time || FRIGN
On Mon, 15 Feb 2016 11:22:08 +0100 Markus Teich wrote: Hey Markus, > why this `argc--, argv++` shenanigans? I think it's more confusing rather than > helping. we came up with this in sbase/ubase as an idiomatic way to set argv0 for tools that don't use arg.h. We need argv0 here because I moved the prepending of the argv[0] into die(), and thus we need to store the argv0 in a global variable. Cheers FRIGN -- FRIGN
Re: [hackers] [slock] Use argv0 instead of passing "slock:" to die every time || FRIGN
g...@suckless.org wrote: > + argv0 = argv[0], argc--, argv++; > - if (argc >= 2 && fork() == 0) { > + if (argc >= 1 && fork() == 0) { > if (dpy) > close(ConnectionNumber(dpy)); > - execvp(argv[1], argv+1); > - die("slock: execvp %s failed: %s\n", argv[1], strerror(errno)); > + execvp(argv[0], argv); > + die("execvp %s failed: %s\n", argv[0], strerror(errno)); Heyho frign, why this `argc--, argv++` shenanigans? I think it's more confusing rather than helping. --Markus
[hackers] [slock] Use argv0 instead of passing "slock:" to die every time || FRIGN
commit b02c4d452a7942d4be3c69e6f98dafd35a2e4e78 Author: FRIGN AuthorDate: Sun Feb 14 02:13:54 2016 +0100 Commit: FRIGN CommitDate: Sun Feb 14 02:13:54 2016 +0100 Use argv0 instead of passing "slock:" to die every time diff --git a/slock.c b/slock.c index 4531f95..a0ffed0 100644 --- a/slock.c +++ b/slock.c @@ -46,6 +46,7 @@ static Bool failure = False; static Bool rr; static int rrevbase; static int rrerrbase; +static char *argv0; static void die(const char *errstr, ...) @@ -53,6 +54,7 @@ die(const char *errstr, ...) va_list ap; va_start(ap, errstr); + fprintf(stderr, "%s: ", argv0); vfprintf(stderr, errstr, ap); va_end(ap); exit(1); @@ -88,9 +90,9 @@ getpw(void) errno = 0; if (!(pw = getpwuid(getuid( { if (errno) - die("slock: getpwuid: %s\n", strerror(errno)); + die("getpwuid: %s\n", strerror(errno)); else - die("slock: cannot retrieve password entry\n"); + die("cannot retrieve password entry\n"); } rval = pw->pw_passwd; @@ -98,7 +100,7 @@ getpw(void) if (rval[0] == 'x' && rval[1] == '\0') { struct spwd *sp; if (!(sp = getspnam(getenv("USER" - die("slock: cannot retrieve shadow entry (make sure to suid or sgid slock)\n"); + die("cannot retrieve shadow entry (make sure to suid or sgid slock)\n"); rval = sp->sp_pwdp; } #endif @@ -106,7 +108,7 @@ getpw(void) /* drop privileges */ if (geteuid() == 0 && ((getegid() != pw->pw_gid && setgid(pw->pw_gid) < 0) || setuid(pw->pw_uid) < 0)) - die("slock: cannot drop privileges\n"); + die("cannot drop privileges\n"); return rval; } #endif @@ -254,7 +256,7 @@ lockscreen(Display *dpy, int screen) usleep(1000); } if (!len) { - fprintf(stderr, "slock: unable to grab mouse pointer for screen %d\n", screen); + fprintf(stderr, "unable to grab mouse pointer for screen %d\n", screen); } else { for (len = 1000; len; len--) { if (XGrabKeyboard(dpy, lock->root, True, GrabModeAsync, GrabModeAsync, CurrentTime) == GrabSuccess) { @@ -264,7 +266,7 @@ lockscreen(Display *dpy, int screen) } usleep(1000); } - fprintf(stderr, "slock: unable to grab keyboard for screen %d\n", screen); + fprintf(stderr, "unable to grab keyboard for screen %d\n", screen); } /* grabbing one of the inputs failed */ running = 0; @@ -281,24 +283,26 @@ main(int argc, char **argv) Display *dpy; int screen; + argv0 = argv[0], argc--, argv++; + #ifdef __linux__ dontkillme(); #endif if (!getpwuid(getuid())) - die("slock: no passwd entry for you\n"); + die("no passwd entry for you\n"); #ifndef HAVE_BSD_AUTH pws = getpw(); #endif if (!(dpy = XOpenDisplay(0))) - die("slock: cannot open display\n"); + die("cannot open display\n"); rr = XRRQueryExtension(dpy, &rrevbase, &rrerrbase); /* Get the number of screens in display "dpy" and blank them all. */ nscreens = ScreenCount(dpy); if (!(locks = malloc(sizeof(Lock*) * nscreens))) - die("slock: malloc: %s\n", strerror(errno)); + die("Out of memory.\n"); int nlocks = 0; for (screen = 0; screen < nscreens; screen++) { if ((locks[screen] = lockscreen(dpy, screen)) != NULL) @@ -313,11 +317,11 @@ main(int argc, char **argv) return 1; } - if (argc >= 2 && fork() == 0) { + if (argc >= 1 && fork() == 0) { if (dpy) close(ConnectionNumber(dpy)); - execvp(argv[1], argv+1); - die("slock: execvp %s failed: %s\n", argv[1], strerror(errno)); + execvp(argv[0], argv); + die("execvp %s failed: %s\n", argv[0], strerror(errno)); } /* Everything is now blank. Now wait for the correct password. */