Introducing a poweroff(8) command

John Baldwin jhb at FreeBSD.org
Mon Aug 23 15:33:48 PDT 2004


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 at FreeBSD.org>  <><  http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve"  =  http://www.FreeBSD.org


More information about the freebsd-arch mailing list