scandir leak patch

Jeff Johnston jjohnstn@redhat.com
Mon Nov 24 23:15:00 GMT 2008


Modified patch checked in.

-- Jeff J.

Jeff Johnston wrote:
> Joel Sherrill wrote:
>> Jeff Johnston wrote:
>>> Joel Sherrill wrote:
>>>  
>>>> Attached is the current newlib scandir.c with
>>>> the "goto cleanup_and_bail" modification from
>>>> the RTEMS one plus an explicit check for 0
>>>> entries from the fstat.
>>>>
>>>> How does this look?
>>>>
>>>> 2008-11-24  Joel Sherrill <joel.sherrill@oarcorp.com>
>>>>
>>>>    * libc/posix/scandir.c: Fix memory leaks.
>>>>
>>>>     
>>> Hi Joel,
>>>
>>>   Thanks.  I did some cleanup of your patch.  First of all you 
>>> should have been using reallocf instead of realloc
>>> since you reset the input pointer.
>> Wasn't it just realloc in CVS?
>> What's the rule to follow?  I don't see reallocf() Linux man pages.
>
> No rules in particular, but realloc won't free up the original pointer 
> if it can't allocate the larger storage.  So, you could use realloc, 
> but since you don't want to keep the original storage if larger can't 
> be allocated, you should use a second variable to put the result in so 
> that you don't lose the original pointer to free.  The reallocf 
> function was only very recently added to newlib.  It's FreeBSD.
>>>   Secondly I wasn't thrilled with the cleanup_and_bail section 
>>> occurring after the normal return so I added a flag to indicate "if 
>>> successful"  and used it so there would only be the one return 
>>> statement.  Let me know if I missed anything or whether you have any 
>>> problems with the modifications.
>>>
>>>   
>> That's fine.  Accomplishes the same thing.  I like yours.  The
>> only solution I don't like is the one that duplicates that code
>> every exit path.
>>>   I have attached a new diff which would be applied to the original 
>>> scandir.c.
>>>
>>>   
>> Looks fine to me.  I am surprised this got noticed and fixed
>> in the RTEMS copy of scandir.c from newlib 1.8 and never
>> got spotted in newlib.  That is a LONG time (16-8 == 8 years!!!)
>>
> Yes.  Good thing we sync up from time to time :)
>>> -- Jeff J.
>>>   
>>
>>
>



More information about the Newlib mailing list