Date: Mon, 23 Aug 2004 17:35:53 -0400 From: John Baldwin <jhb@FreeBSD.org> To: freebsd-arch@FreeBSD.org Cc: Stefan Farfeleder <stefanf@FreeBSD.org> Subject: Re: Introducing a poweroff(8) command Message-ID: <200408231735.53831.jhb@FreeBSD.org> In-Reply-To: <20040823210944.GC91753@wombat.fafoe.narf.at> References: <20040821191659.GA94336@gothmog.gr> <200408231141.25077.jhb@FreeBSD.org> <20040823210944.GC91753@wombat.fafoe.narf.at>
next in thread | previous in thread | raw e-mail | index | archive | help
On Monday 23 August 2004 05:09 pm, Stefan Farfeleder wrote:
> On Mon, Aug 23, 2004 at 11:41:24AM -0400, John Baldwin wrote:
> > On Saturday 21 August 2004 04:22 pm, Giorgos Keramidas wrote:
> > > - if (strstr((p = rindex(*argv, '/')) ? p + 1 : *argv, "halt")) {
> > > + p = rindex(*argv, '/') ? p + 1 : *argv;
> > > + if (strcmp(p, "halt") == 0) {
> >
> > I think this is buggy in that p will point to the / character since you
> > don't modify it in the second case. I.e. what you wrote is basically
> > this:
> >
> > p = rindex(*argv, '/');
> > if (p != NULL)
> > p + 1; /* does nothing */
> > else
> > *argv; /* also does nothing */
>
> No,
> p = rindex(*argv, '/') ? p + 1 : *argv
> is parsed as
> p = (rindex(*argv, '/') ? p + 1 : *argv).
> Your code is equivalent to
> (p = rindex(*argv, '/')) ? p + 1 : *argv.
In that case p's value is undefined when you do p + 1 and it's still a very
broken piece of code. The deleted if statement from my original e-mail is
not broken. :) It is true that = is parsed last though, I just misparsed
operator(7). I wonder if the original code worked if you passed the full
path since it would need to be your 3rd alternative. Actually, if you look
at the original code, it did work as it used your 3rd alternative:
- if (strstr((p = rindex(*argv, '/')) ? p + 1 : *argv, "halt")) {
So my point about the new code having a but still stands, and to me it shows
that the use of ?: instead of if-else only serves to obfuscate the code and
lead to bugs.
--
John Baldwin <jhb@FreeBSD.org> <>< http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve" = http://www.FreeBSD.org
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200408231735.53831.jhb>