On Fri, Aug 30, 2013 at 2:30 AM, Terry Ellison <ellison.te...@gmail.com>wrote:
> On 27/08/13 10:40, Nikita Popov wrote: > >> On Sat, Aug 3, 2013 at 8:16 PM, Nikita Popov <nikita....@gmail.com> >> wrote: >> >> Hi internals! >>> >>> Is there any particular reason why we only pass return_value_ptr to >>> internal functions if they have the ACC_RETURN_REFERENCE flag set? >>> >>> Why can't we always provide the retval ptr, even for functions that don't >>> return by reference? This would allow returning zvals without having to >>> copy them first (what RETVAL_ZVAL does). >>> >>> Motivation for this is the following SO question: >>> http://stackoverflow.com/q/**17844379/385378<http://stackoverflow.com/q/17844379/385378> >>> >>> Patch for this change can be found here: >> https://github.com/php/php-**src/pull/420<https://github.com/php/php-src/pull/420> >> >> The patch also adds new macros to allow easy use of this feature called >> RETVAL_ZVAL_FAST/RETURN_ZVAL_**FAST (anyone got a better name?) >> >> If no one objects I'll merge this sometime soon. >> > +1 Though looking through the ext uses, most functions returning an array > build it directly in return_value and thus avoid the copy. I also see that > you've picked up all of the cases in ext/standard/array.c where these > macros can be applied. > > There's another one in string.c, in PHP_FUNCTION(pathinfo), that could be > applied as well, though there's little performance gain in avoiding the > copy of a 4 element string array. > > BTW, looking at this pathinfo code, it doesn't do what the documentation > says it does -- or at least this states that the optional argument if > present should be _one_ of PATHINFO_DIRNAME, PATHINFO_BASENAME, > PATHINFO_EXTENSION or PATHINFO_FILENAME. However, if a bitmask is supplied > then this function returns the element corresponding to the lowest bit > value rather than an error return, for example: > > $ php -r 'echo pathinfo("/tmp/x.fred", PATHINFO_FILENAME|PATHINFO_** > EXTENSION),"\n";' > fred > > This is a bizarre behaviour. At a minimum the documentation should > actually state what the function does. Or do we bother to raise a patch to > fix this sort of thing, given that returning an empty string (or more > consistently with other functions, NULL) in this case could create a BC > break with existing buggy code? > This is weird, yes. It's not the lowest bit value that is returned, but the first element put in the array (as zend_hash_get_current_data() is used with no HashPosition) , which is even more confusing. How to explain that in the documentation ? :| Julien.Pauli
