DragonFly BSD
DragonFly commits List (threaded) for 2004-12
[Date Prev][Date Next]  [Thread Prev][Thread Next]  [Date Index][Thread Index]

Re: cvs commit: src/usr.bin/newgrp

From: Max Okumoto <okumoto@xxxxxxxx>
Date: Mon, 13 Dec 2004 00:36:43 -0800

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.

Matthew Dillon <dillon@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.


[Date Prev][Date Next]  [Thread Prev][Thread Next]  [Date Index][Thread Index]