There's already ZEND_RESULT_CODE, or did I miss anything? On 25 Dec 2014 06:45, "Xinchen Hui" <larue...@php.net> wrote:
> Hey: > > On Thu, Dec 25, 2014 at 12:38 PM, Pierre Joye <pierre....@gmail.com> > wrote: > > On Thu, Dec 25, 2014 at 3:06 PM, Andrea Faulds <a...@ajf.me> wrote: > >> > >>> On 24 Dec 2014, at 23:53, Levi Morrison <le...@php.net> wrote: > >>> > >>> On Wed, Dec 24, 2014 at 4:27 PM, Johannes Schlüter > >>> <johan...@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. > yes. and as a soft solution. > > we can change these functions which use success/failure return > zend_status instead of int first. > > thanks > > > > > Cheers, > > -- > > Pierre > > > > @pierrejoye | http://www.libgd.org > > > > -- > Xinchen Hui > @Laruence > http://www.laruence.com/ > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php > >