On Thu, Jan 15, 2026 at 03:54:54PM -0800, Marc Herbert wrote:
> Hi Alison,
>
> Alison Schofield <[email protected]> writes:
> > argc = parse_options(argc, argv, options, u, 0);
> > - if (argc > 0)
> > - device = basename(argv[0]);
> > + if (argc > 0) {
> > + device = strrchr(argv[0], '/');
> > + device = device ? device + 1 : argv[0];
> > + }
> >
>
>
> 1. I would add a one-line comment in both places, something like "This
> is like basename but without the bugs and portability issues" because:
>
> 1.a) It's much faster to read such a comment than understanding the code.
> 1.b) Not everyone knows how much of GNU/POSIX disaster is "basename".
> You summarized it well in the commit message but it's unlikely
> anyone will fetch the commit message from git without such a comment.
>
> To avoid duplicating the comment, a small "my_basename()" inline
> function would not hurt while at it.
Thanks for the review.
I'm headed down the Dan suggested path of adding a helper.
>
>
> 2. I believe this (unlike basename) returns an empty string when the
> argument has a trailing slash. Now, an argument with a trailing slash
> would probably be garbage and I'm OK with the "Garbage IN, garbage
> OUT" principle. BUT I also believe in the "Proportionate Response"
> principle, which means a small amount of garbage IN should IMHO not
> be punished by some utterly baffling error message or (much worse) a
> crash. Did you/could you test what happens with a trailing slash? If
> the resulting failure makes some kind of sense then don't change
> anything.
With the implementation moved to a new helper, path_basename()
I've tested these:
assert(strcmp(path_basename("/usr/bin/foo"), "foo") == 0);
assert(strcmp(path_basename("foo"), "foo") == 0);
assert(strcmp(path_basename("/usr/bin/foo/"), "") == 0);
assert(strcmp(path_basename("/"), "") == 0);
assert(strcmp(path_basename(""), "") == 0);
I think this sticks w gigo principle and 'proportionate response' too.
It's safe for valid paths, safe for unusual but harmless paths, no
crashes or undefined behavior, and the thing that inspired this, is
that it behave the same across libc implementations.
Watch for the v2 and let me know if i missed any string tests above.
>
> Note even when rare in interactive use, "Garbage IN" becomes more
> frequent in automation. Then good luck making sense of a cryptic error
> when you can't even reproduce the elaborate test configuration and test
> bugs that trigger it.
>
