Standard C functions: Check for -1 or 0?
Asked Answered
P

9

5

Many standard C and POSIX functions return -1 for error, and 0 on success, for example truncate, fflush, msync, etc.

int ret = truncate("/some/file", 42);

Is it better practice to check for success with ret != -1 or ret == 0, and why?

My Thoughts

It's my experience most people check for the error case (ret != -1), as there is typically only one (consider functions that return NULL or EOF on error). However in hindsight, one could see these functions could benefit from returning errno directly (where 0 is considered no error).

There is also the worry that a function returns something other than 0 or -1, or that additional return values are later added. In these scenarios, it makes sense to test for the "tightest" range of values indicating success (ret == 0).

Update0

It's an assumption of mine that people are aware that EOF is typically defined as -1.

Porta answered 1/10, 2010 at 4:38 Comment(1)
According to my documentation fflush does not return -1 on failure (it returns EOF). But best to use whichever is most logical for the situation at hand. Having rules without context is not really usefull.Roa
S
3

In my opinion, it really depends on what you need to do and the range of return values.

Take a call with one success value and many failure values. It's generally easier to == or != against the successful return value than check against any failure values. In this case, you would test against != success if you need to, say, log and return or throw in case of a failure.

In a call with one and one, the desired behavior is more important, so I suggest choosing the more readable. If your codes needs to react to and possibly return on failure, then check for == failure instead of != success. The former is more readable, since it takes one step of thinking and the failure constant may be helpfully named.

You may also want to consider which is more likely and handle that first, or which is numerically greater or lesser.

In your case, with two functions sharing a success code and with differing failure codes, it falls to whichever seems more readable. I would agree that testing for == 0 in both cases would seem better, but it really depends on how much code comes between the calls. If you have a few dozen lines, there might not be much of a difference in readability. If it's very close, you might even be able to OR the results together and save yourself a few more steps.

Sabulous answered 1/10, 2010 at 4:53 Comment(2)
+1: I like this. Just as boolean variables and filenames should be "positive" assertions like is_empty() or is_full() rather than not_empty(), cant_connect() - it's more intuitively sensible to me to see tests for error as != _success_ test as the != itself hints at an error of some kind, same as == success reads more smoothly than != failure. Boolean logics simpler to get right without extra negations and double-negations. I prefer if (... < 0) over if (... == -1): the latter implies something more counter-intuitive while the first raises "honest" questions.Mullens
Definitely. In my opinion, since there's no real performance difference or difficulty in writing this code, it comes down to which is more readable. You could even look into using constants for this particular situation to make things even clearer (and easier if you ever change calls).Sabulous
R
8

It depends on whether the function is a C standard library function or a POSIX function. C functions don't have a uniform standard for return codes so I'd use whatever test makes the most sense on a case-by-case basis.

POSIX, on the other hand, is more consistent. Almost all POSIX functions are defined to return -1 with a more specific error code available in errno. Some functions simply return 0 for success, while others have a multitude of success values. For example, open() returns file descriptors, read() returns the number of bytes read, etc.

For consistency I like to always use the same test when invoking POSIX functions: don't test for success, test for failure. POSIX functions that today return -1 upon error will always return exactly -1, so I would use one of two checks for all of them:

if (ret == -1) {
    perror(...);
}

// or

if (ret < 0) {
    perror(...);
}

(I prefer the first one but the second more general one doesn't bother me.)

Roumell answered 1/10, 2010 at 4:50 Comment(7)
The second one is a teeny-tiny bit faster on most architectures, since there's usually a status flag for negativity and conditional branches which use it.Reef
It's not true that "POSIX functions now and always will return exactly -1 on error" - see for example the POSIX definition of getpwnam_r(), which says "an error number shall be returned" on error.Sundstrom
@caf: It's precisely your treatment of getpwnam_r() that led me to this question. If anything, that function deserves a question of its own, it appears to simultaneously return success or error in both the return value, AND the pwbufp fields.Porta
@Ben Voigt, @John Kugelman: I'm aware that one can assume that the positive successful return values are not likely to wrap around into the negative area of functions that return signed values. But it's possible that they could? For this reason I avoid grandiose assumptions that any negative value can be treated as an error. What are your thoughts?Porta
@Matt While that is not going to happen, that's exactly why I prefer the first form. The generalization to ret < 0 is unnecessary and can raise doubts like yours while ret == -1 is unambiguously correct.Roumell
@Matt Joiner: It's a bit weird, but "user not found" is not treated as failure - the function returns 0, but pwbufp is NULL. This is so you can tell the difference between "user definitely doesn't exist" and "failure to determine if user exists or not".Sundstrom
Is there a reason not to generalize the second form further to if (ret != 0)?Poulos
B
3

Comparing the man pages for truncate and fflush, both return 0 on success, but return different values for error (truncate -> -1, fflush -> EOF). So I'd check for 0.

Burka answered 1/10, 2010 at 4:45 Comment(1)
There are gobs of functions for which positive values mean success, such as open, read, write, the list goes on and on.Reef
S
3

In my opinion, it really depends on what you need to do and the range of return values.

Take a call with one success value and many failure values. It's generally easier to == or != against the successful return value than check against any failure values. In this case, you would test against != success if you need to, say, log and return or throw in case of a failure.

In a call with one and one, the desired behavior is more important, so I suggest choosing the more readable. If your codes needs to react to and possibly return on failure, then check for == failure instead of != success. The former is more readable, since it takes one step of thinking and the failure constant may be helpfully named.

You may also want to consider which is more likely and handle that first, or which is numerically greater or lesser.

In your case, with two functions sharing a success code and with differing failure codes, it falls to whichever seems more readable. I would agree that testing for == 0 in both cases would seem better, but it really depends on how much code comes between the calls. If you have a few dozen lines, there might not be much of a difference in readability. If it's very close, you might even be able to OR the results together and save yourself a few more steps.

Sabulous answered 1/10, 2010 at 4:53 Comment(2)
+1: I like this. Just as boolean variables and filenames should be "positive" assertions like is_empty() or is_full() rather than not_empty(), cant_connect() - it's more intuitively sensible to me to see tests for error as != _success_ test as the != itself hints at an error of some kind, same as == success reads more smoothly than != failure. Boolean logics simpler to get right without extra negations and double-negations. I prefer if (... < 0) over if (... == -1): the latter implies something more counter-intuitive while the first raises "honest" questions.Mullens
Definitely. In my opinion, since there's no real performance difference or difficulty in writing this code, it comes down to which is more readable. You could even look into using constants for this particular situation to make things even clearer (and easier if you ever change calls).Sabulous
D
2

Cert guidelines seem to prefer the '!= 0' check as is valid in many of their example code snippets.

Discrepant answered 1/10, 2010 at 4:47 Comment(0)
D
2

Always check the man pages for return codes.

Usually 0 means success, but exceptions exist such as printf().

Check man 2 intro for errnos and error codes of system calls.

Dotation answered 1/10, 2010 at 4:52 Comment(1)
Quite useful, didn't know of that page.Porta
M
2

Whatever you do, never ever shortcut a test for success with

if (!ret) 

It's confusing and someone (including yourself) will misread it later as a test for failure. It's generally better to use explicit tests.

Martsen answered 1/10, 2010 at 7:32 Comment(0)
P
1

If the definition is that 0 means success, and you want to check for success then you should check equivalence to 0. (and this is for no other reason than shear readability)

Peashooter answered 1/10, 2010 at 4:46 Comment(0)
R
1

For most POSIX API functions, negative values are errors. So I'd test for failure with if (ret < 0).

Reef answered 1/10, 2010 at 4:47 Comment(3)
@Mark: Yeah, I guess I should state that more clearly. In my experience, usually you test for failure, and the success case simply doesn't get an early exit.Reef
That's my experience too, but the OP is specifically asking for what the "right" check for success would look likePeashooter
Can you specify a POSIX function that returns more than one negative error value (such as -1)?Porta
A
0

My rule of thumb is that functions like these return the error code (or just whether an error occurred) through the return value. So, for me it makes sense that a return value of 0 means that there was nothing of that sort to return and that no error occurred. Therefore I just check if the return value was 0 if I want to test whether the function was successful, and if not just check what the error value was and deal with it accordingly.

Autograft answered 1/10, 2010 at 4:55 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.