DragonFly commits List (threaded) for 2004-12
Re: cvs commit: src/usr.bin/newgrp
Devon H. O'Dell wrote:
Matthew Dillon wrote:
:liamfoy 2004/12/08 14:00:32 PST
:DragonFly src repository
: Modified files:
: usr.bin/newgrp newgrp.c : Log:
: - Check the return value of setenv(). We should check this value since
: setenv() uses both malloc and realloc.
: : Revision Changes Path
: 1.2 +9 -3 src/usr.bin/newgrp/newgrp.c
That's fine, though the typical way of checking the return
value is to check for it < 0 rather then exactly equal to
-1. Both are correct, but < 0 is used the most often in the code
base and considered easier to read.
Is it? I always found == -1 to be rather straightforward, and
considering (according to SUSv3) that setenv _must_ return only 0 or -1,
I see no point to check for any other value. If setenv were to return
anything else on failure, and you would need to check what the error
was, I could understand this. But errno is set when setenv errors, and
returns -1, so I'm stumped as to why checking for < 0 is considered to
be easier to read, since it seems more like an obfuscation to what the
case actually can be in reality.
Devon H. O'Dell
Using < 0 tells me that if any error value is returned, handle it here.
Alot of the code that I have seen uses the < 0 idiom. Negative numbers
are commonly used to indicate error, but in most cases it just dosen't
matter which error has occured. Testing of == indicates to me that
you are testing for a spacific error. But doing that slows down my
reading of the code since now I have to see which other errors might
be returned. Yes you know that setenv() and alot of other functions
return only -1, but some return other negative numbers. So now I have
to go check if setenv() returns other errors.