Re: [hackers] [slock] Use argv0 instead of passing "slock:" to die every time || FRIGN

2016-02-15 Thread Anselm R Garbe
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

2016-02-15 Thread Dimitris Papastamos
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

2016-02-15 Thread Markus Teich
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

2016-02-15 Thread 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

2016-02-15 Thread 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

2016-02-15 Thread Markus Teich
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