Re: [PATCH] likely cleanup: remove unlikely for kfree(NULL)

[Kyle Moffett <mrmacman_g4@mac.com>]

On Apr 27, 2006, at 04:17:50, Bart Hartgers wrote:
> Kyle Moffett wrote:
>> static inline void kfree(void *ptr)
>> {
>>     if (__builtin_constant_p((ptr == NULL))) {
>>         if (ptr)
>>             kfree_nonnull(ptr);
>>     } else {
>>         kfree_unknown(ptr);
>>     }
>> }
>>
>> void kfree_nonnull(void *ptr)
>> {
>>     /* kfree code here, no null check */
>> }
>>
>> void kfree_unknown(void *ptr)
>> {
>>     if (ptr)
>>         kfree_nonnull(ptr);
>> }
>
> I still think there is an inconsistency in gcc. If I call your  
> kfree with the following:
>
> void test( char *ptr )
> {
>         char *null = NULL;
>         kfree(ptr);	/* unknown */
>         *ptr = 'a';
>         kfree(ptr);	/* nonnull */
>         kfree(null);	/* should be optimised away */
> }
>
> ,the compiler (4.1) generates two calls to kfree_unknown instead of  
> one to kfree_nonnull and one to kfree_unknown. It seems that the  
> __builtin_constant_p((ptr==NULL)) check does not always trigger,  
> even if the compiler 'knows' ptr to be equal to NULL. I posted a  
> nasty hack around this problem yesterday.

I know.  You can "fix" this problem by changing the if statement to  
this:

if (__builtin_constant_p(ptr) || __builtin_constant_p((ptr == NULL)))

On the other hand, calling kfree(ptr) on a non-NULL constant pointer  
is a bug and will crash, and calling kfree(ptr) on a NULL constant  
ptr is just dead code and we should find and kill that separately.   
There's no reason to ever call kfree(<constant>).

Cheers,
Kyle Moffett