Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:95115
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmx.de designates 212.227.17.21 as permitted sender)
To: Nikita Popov <nikita.ppv@gmail.com>
References: <85e38ba1-58d4-f8a7-33d4-5f4aa7fce44d@gmx.de>
 <CAF+90c9oV1mV-iDs4qZ0x-NLqrtg8piJ0uY2c2+gyJfMrVN9UA@mail.gmail.com>
Cc: PHP internals <internals@lists.php.net>
Message-ID: <23d3655e-2686-f3d8-c28a-496b95b33d03@gmx.de>
Date: Sat, 13 Aug 2016 15:35:00 +0200
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101
 Thunderbird/45.2.0
MIME-Version: 1.0
In-Reply-To: <CAF+90c9oV1mV-iDs4qZ0x-NLqrtg8piJ0uY2c2+gyJfMrVN9UA@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Subject: Re: Bug #72828
From: cmbecker69@gmx.de ("Christoph M. Becker")

On 13.08.2016 at 15:08, Nikita Popov wrote:

> On Sat, Aug 13, 2016 at 2:42 PM, Christoph M. Becker <cmbecker69@gmx.de>
> wrote:
> 
>> You have commented on <https://bugs.php.net/bug.php?id=72828>:
>>
>> | Unless the allocations explicitly use the system allocator (i.e. do
>> | not use emalloc and variants), do NOT introduce NULL checks.
>>
>> Can you please elaborate, why that shouldn't be done.
>>
>> Actually, the allocations use safe_emalloc() and emalloc(),
>> respectively[1].  However, the only client of the function does
>> explicitly check for a NULL return[2], which can only happen, if
>> safe_emalloc() fails.  So if no NULL checks should be done, this one
>> should be removed as well.
>>
>> [1]
>> <https://github.com/php/php-src/blob/php-5.6.24/ext/
>> standard/string.c#L2926-L2927>
>> [2]
>> <https://github.com/php/php-src/blob/php-5.6.24/ext/
>> standard/string.c#L3133-L3134>
> 
> If that's the only failure condition of the function, the check can indeed
> be dropped.

Fine; I'll do that.

> ZMM is an infallible allocator, it will bail out (or abort) if the
> allocation fails. The return value should never be checked -- if it is
> checked, it's a mistake.

Thanks for the explanation!

> If you are concerned about USE_ZEND_ALLOC=0 using a fallible allocator, the
> system allocator hooks in ZMM should be adjusted to check for NULL and
> abort instead (use __zend_malloc instead of malloc, for example).

I don't know whether USE_ZEND_ALLOC=0 is in use on any production
systems, but if that is so, that should be catered to.  And maybe also
to zero size allocations, which have implemention defined behavior
(either NULL or a valid pointer may be returned).

-- 
Christoph M. Becker