cvs commit: src/usr.bin/newgrp

Max Okumoto okumoto at ucsd.edu
Mon Dec 13 00:36:43 PST 2004


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.

                    -Matt
                    Matthew Dillon                     
<dillon at xxxxxxxxxxxxx>
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.

Kind regards,

Devon H. O'Dell
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.
Using < 0 tells me that if any error value is returned, handle it here.

					Max






More information about the Commits mailing list