Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:4212
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Reply-To: zeev@zend.com
Message-ID: <5.1.0.14.2.20030828020639.0596b988@localhost>
Date: Thu, 28 Aug 2003 02:15:00 +0300
To: Ard Biesheuvel <abies@php.net>
Cc: internals@lists.php.net
In-Reply-To: <20030827204254.35421.qmail@pb1.pair.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; format=flowed
Subject: Re: [PHP-DEV] [PATCH] __attribute_malloc__ for malloc-like
  functions
From: zeev@zend.com (Zeev Suraski)
References: <20030827204254.35421.qmail@pb1.pair.com>

At 23:43 27/08/2003, Ard Biesheuvel wrote:
>The other thing is, that a lot of warnings are being triggered by 
>invocations of zend_hash_find(). This function stores its result in the 
>location pointed to by a void** argument, and returns an int specifying 
>whether the key was found or not. As the comment in the source states, 
>this was a conscious decision, because hashes can also be used to store 
>null pointers.
>
>After looking through some of the code, I found out that in a lot of 
>cases, the pointers are expected not to be zero, and are being 
>dereferenced in the code that immediately follows it [without checking for 
>validity first].
>
>Wouldn't it be more straight-forward to introduce an analogue for 
>zend_hash_find() (eg. 'void *zend_hash_get()') which returns the stored 
>pointer [or NULL on failure]. It could be used for applications where null 
>pointers aren't allowed, and fix current code, where the implicit 
>assumption is made the SUCCESS means the the pointers is non-null.
>
>Last of all, I would like to know if we care about the warnings. 
>Personally I don't really mind, as GCC's capabilities of accurately 
>identifying violations of strict-aliasing rules are limited anyway.

I'd just make gcc shut up about it.  Using void ** is the generic way to do 
what we want to do and I see no reason to introduce another way that works 
only in a subset of the cases, and will require tons of code changes.  I 
have to admit I still haven't been able to figure out what gcc thinks is 
wrong with casting a zval *** pointer to a void **, considering that any 
pointer can be cast to a void *, and that pointers always have the same 
size (i.e., any pointer is castable to void *).

Not sure about __attribute_malloc__ - it sounds reasonable but I have no 
experience with it.  Were you able to measure any performance difference?

Zeev