[Ocfs2-tools-devel] [PATCH] Fix memleak while calculating number of extents

[Tao Ma]

Hi Goldwyn,
Goldwyn Rodrigues wrote:
> On Wed, Aug 18, 2010 at 5:25 PM, Sunil Mushran <sunil.mushran at oracle.com> wrote:
>   
>> On 08/18/2010 10:09 AM, Goldwyn Rodrigues wrote:
>>     
>>> While calculating number of extents, the buffers are allocated in a loop
>>> but only the last buffer is freed in the end. Fix the memleak by freeing
>>> the buffer inside the loop.
>>>
>>> Found by Joel Becker while reviewing defrag.ocfs2
>>>
>>> Signed-off-by: Goldwyn Rodrigues<rgoldwyn at suse.de>
>>> ---
>>> diff --git a/debugfs.ocfs2/commands.c b/debugfs.ocfs2/commands.c
>>> index 1f60049..ae12479 100644
>>> --- a/debugfs.ocfs2/commands.c
>>> +++ b/debugfs.ocfs2/commands.c
>>> @@ -2092,6 +2092,7 @@ static errcode_t calc_num_extents(ocfs2_filesys *fs,
>>>                        if (ret)
>>>                                goto bail;
>>>
>>> +                       ocfs2_free(&buf);
>>>                }
>>>
>>>                *ne = *ne + extents;
>>>
>>>
>>>       
>> Correct. But the ocfs2_free() after bail needs to be removed.
>>
>>     
>
> No, what if ocfs2_read_extent_block fails? You would still get a leak.
> The ocfs2_free after bail is conditional with respect to buf, so it seems fine.
>   
I think you need to reset 'buf = NULL' after you call ocfs2_free 
otherwise if the following ocfs2_malloc_block fails, you
will double free the same buf.

But I think the right solution is lift the ocfs2_malloc_block and 
ocfs2_free out of the loop.
I see you just use it iterately in the loop, so why not allocating it at 
the beginning of calc_num_extents
and free in the end?

Regards,
Tao