On Sun, Dec 04, 2022 at 03:19:18PM -0700, Todd C. Miller wrote:
> On Sun, 04 Dec 2022 15:58:22 -0600, Scott Cheloha wrote:
>
> > On Mon, Dec 05, 2022 at 06:37:49AM +1000, David Gwynne wrote:
> > > On Sat, Dec 03, 2022 at 08:14:37PM -0600, Scott Cheloha wrote:
> > > > There are nearly a hundred vestigial question mark cases in the
> > > > top-level getopt(3) loops of various programs. You know, this:
> > > >
> > > > switch (ch) {
> > > > /* ... */
> > > > case '?':
> > > > default:
> > > > usage();
> > > > }
> > > >
> > > > All of them are redundant and can be handled by the "default" case,
> > > > which invariably fails the program.
> > > >
> > > > This patch covers bin, games, regress, sbin, usr.bin, and usr.sbin. I
> > > > excluded the csh(1)/ksh(1) builtins. I also excluded nsd and unbound;
> > > > they don't look domestic. gnu is also not domestic and is excluded.
> > > >
> > > > ok?
> > >
> > > mostly ok. can md5.c keep the switch statement and use default instead
> > > of case '?'? everything else looks good.
> >
> > md5 is an odd one. There are two getopt(3) loops. One for the -b
> > flag and a second loop for everything else.
> >
> > I think the switch statement in the first loop, the -b flag loop, is
> > misleading. We're only looking for the -b flag in that loop, so why
> > do we have a switch statement? The second loop handles unexpected
> > flags in the usual way.
> >
> > millert: any preferences?
>
> The problem with replacing the switch() with an if() there is that
> "unknown option" warnings will be printed twice. This can be avoided
> by making the option string start with a ':' for the first getopt()
> loop, which is how I originally fixed this (using an if() statement).
> It was changed to a switch() with the '?' label later on by sobrado@.
>
> It is probably simplest to just do what dlg@ suggests and change
> the case '?' to default.
No, we can't do that, because then we usage() anything that isn't the
-b flag in the first loop.
I'm just going to leave md5.c as-is.
Sorry for the noise.
