On Sun, Feb 19, 2023, at 09:21, Max Kellermann wrote: > On 2023/02/19 08:56, Nikita Popov <nikita....@gmail.com> wrote: > > If you have a function like zend_stream_open_function(), SUCCESS and > > FAILURE are directly meaningful values. > > Agree, but that doesn't explain why FAILURE needs to be negative.
I expect that there are two main reasons for that: - There are probably some places that return a (non-negative) value or FAILURE. - There are probably some places that check for success/failure using >= 0 and < 0. Another POSIX-ism. I don't think we endorse such usage, but it likely exists. Let me turn the question around: Is there some reason to change the value of FAILURE at this point? > > The current guideline for use of bool and zend_result in php-src is that > > bool is an appropriate return value for "is" or "has" style functions, > > which return a yes/no answer. zend_result is an appropriate return value > > for functions that perform some operation that may succeed or fail. > > What does the return value of these functions mean? > > - zend_make_printable_zval() > - zend_make_callable() > - zend_parse_arg_bool() > - zend_fiber_init_context() > - zend_observer_remove_begin_handler() > - php_execute_script()1 > > If I understand the guideline correctly, then those examples (and > countless others) are defective and should be fixed, correct? At least in principle, yes. Of course, actually doing a change may not always be worthwhile, especially as this might result in a silent behavior change for extensions (as putting the return value into an "if" would invert behavior now). I believe Girgias has done extensive work on making the int vs bool vs zend_result situation more consistent, so you might want to coordinate with him. Regards, Nikita
