Re: About SUCCESS/FAILURE

On Thu, Dec 25, 2014 at 3:06 PM, Andrea Faulds <ajf@ajf.me> wrote: > >> On 24 Dec 2014, at 23:53, Levi Morrison <levim@php.net> wrote: >> >> On Wed, Dec 24, 2014 at 4:27 PM, Johannes Schlüter >> <johannes@schlueters.de> wrote: >>> On Wed, 2014-12-24 at 11:13 -0700, Levi Morrison wrote: >>> >>>> I'm asking for specific things. The reason is that some API's do a >>>> non-zero error code; the fact that they are negative is a detail that >>>> we should not need to care about. >>> >>> My guess is that positive values more often might have a meaning ("5 >>> items changed", "address 0x1234") whereas negative values less often >>> have a meaning. Also passing -1 as parameter is more often invalid. Thus >>> passing -1 is making debug output look more suspicious. >>> >>> (while there are cases where -1 is valid, see recent famous pid >>> = fork(); /* ... */ kill(pid, SIGKILL); issue) >> >> I don't think this is the same use case as SUCCESS and FAILURE. Many >> functions have an out parameter which is only valid when the returned >> value is SUCCESS. This is not the same thing as an API which returns >> an integer and just happen to embed error state in the negative range. >> Notably, it doesn't make sense to do strpos() == >> SUCCESS to check >> success; these are different cases. My question is specifically >> directed at the ones that use SUCCESS and FAILURE: which ones require >> FAILURE to be negative instead of the normal UNIX-ism of non-zero? >> >> For the record I am in favor of an enum such as zend_status >> or some >> other name which indicates whether an operation succeeded or not for >> the reasons already cited in this thread. I just don't see why FAILURE >> needs to be negative and want to know why this is the case. > > Hi Levi, > > Again, I think the reason FAILURE is -1 is for consistency with other functions which use > negative return values on error. Some functions return negative error codes, others just -1. Some > functions return useful positive values, others just 0. But the idea is that all functions return a > negative number on error, so you can use if (foo() < 0) to check for errors. That’s the point > of making FAILURE be -1, AIUI. It makes it consistent with other things, like fork() or strpos(). doing if (foo() < 0 is exactly what should not be done, for any function returning a status. Only FAILURE and SUCCESS should be used. Which value FAILURE and SUCCESS have is not really relevant here but to actually be consistent. For example ZEND_API int zend_hash_del(HashTable *ht, zend_string *key) should actually be ZEND_API status zend_hash_del(HashTable *ht, zend_string *key) and its usage should be: if (zend_hash_del(ht, key) == FAILURE) { ... } Same for zend_parse_parameters and the likes. However functions like zval_update_class_constant (http://lxr.php.net/xref/PHP_TRUNK/Zend/zend_API.c#1132 ) and all the underlying functions, are confusing. Both the signature and the return values should rely on FAILURE/SUCCESS. I think this is what Xinchen means too. Or at least this is what I mean with unify the APIs. Cheers, -- Pierre @pierrejoye | http://www.libgd.org