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.
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.
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.
