Bug #72828

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

Hi!

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

These do not need null checks. If e* functions can't alloc memory, they
produce fatal error, bail out and do not return. So if it returned, it
succeeded. The only reason why it can return null is a bug. In this case
we'd prefer it crashing fast - bugs in memory allocator are hard to
find, and closer to the source the crash is, easier it to catch it.

If pe* or system functions are used, then check may be warranted.

Thanks!

I had already closed the bug report, because the issue affects only PHP
5.6, and I don't think this extraneous NULL check is a real bug. Please
re-open if you don't agree, in which case I'll remove the NULL check.

--
Christoph M. Becker