On Thu, Dec 25, 2014 at 3:06 PM, Andrea Faulds <[email protected]> wrote:
>
>> On 24 Dec 2014, at 23:53, Levi Morrison <[email protected]> wrote:
>>
>> On Wed, Dec 24, 2014 at 4:27 PM, Johannes Schlüter
>> <[email protected]> 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
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
