Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:68692
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 74.125.82.43 as permitted sender)
Message-ID: <521FE78E.3060900@gmail.com>
Date: Fri, 30 Aug 2013 01:30:06 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130803 Thunderbird/17.0.8
MIME-Version: 1.0
To: Nikita Popov <nikita.ppv@gmail.com>
CC: PHP internals <internals@lists.php.net>
References: <CAF+90c_i7bU0CGPiA4pgL+d+C+Py1279DTYRivQnSorhc9oq7Q@mail.gmail.com> <CAF+90c8pnaj1s_tsYDZz5G3G6uD2J0CNdRh1yhB_TWSNhZgxdw@mail.gmail.com>
In-Reply-To: <CAF+90c8pnaj1s_tsYDZz5G3G6uD2J0CNdRh1yhB_TWSNhZgxdw@mail.gmail.com>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] Re: Always set return_value_ptr?
From: ellison.terry@gmail.com (Terry Ellison)

On 27/08/13 10:40, Nikita Popov wrote:
> On Sat, Aug 3, 2013 at 8:16 PM, Nikita Popov <nikita.ppv@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
>>
> Patch for this change can be found here:
> 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?

Regards
Terry