On 04/16/2010 03:11 PM, Stephen Gallagher wrote:
On 04/16/2010 07:07 AM, Eugene Indenbom wrote:
> On 04/16/2010 03:00 PM, Stephen Gallagher wrote:
>
>> On 04/16/2010 02:02 AM, Eugene Indenbom wrote:
>>
>>
>>> The current implementation of talloc_zfree macro has a substantial
>>> deficiency: it deallocates memory first and then clears a pointer.
>>>
>>> Clearing pointer after deallocation completes could lead to bad side
>>> effects when callbacks are called during object destruction: the
>>> callback called from destructor could dereference half-destroyed object
>>> referenced by the yet-not-cleared pointer.
>>>
>>> The above could lead to disastrous effects including memory double free.
>>>
>>> The patch attached fixes the issues by using correct free sequence:
>>> clear pointer first, then deallocate memory block.
>>>
>>> Eugene
>>>
>>>
>>>
>> My only comment with this patch is that I don't understand why you use
>> (ptr) instead of just ptr.
>>
>> Otherwise, this patch seems intuitively correct, and I like it.
>>
>>
>>
> That is some kind of macro writing paranoia: always to surround macro
> parameters with parentheses. It should be OK without them, but...
>
>
It would be my preference to leave the parentheses out when not needed.
As someone who does both C and Python programming, I looked at this and
was trying to figure out at first why you were using a tuple here :)
Since this is safe without the parens, I think it would be more readable
to do without.
But as I said before: functionally I approve this patch.
Patch without extra parenthesis is attached.
Eugene