Stefan Huelswitt wrote:
On 29 Mar 2005 Daniel THOMPSON daniel.thompson@st.com wrote:
Hi,
There are routes through tComponent::FromString() that explicitly set description to NULL without checking its value first (when n != 4). This appears to me to be the leak.
Running the following code it is obvious that glibc malloc'ed desc even when desc is not converted.
Wow, this is cool. How did you get the idea to search in that direction?
I got the idea from the reading your patch and the premise that it was unlikely that there was a bug in glibc.
Put simply I *never* blame core software like glibc or the compiler for bugs unless it is proved to me. These bits of software are so widely used that while blaming them is not *always* wrong it usually saves a lot of time to audit your own code first. Also I've met Ulrich Drepper and wouldn't want to let him catch me blaming glibc for something it didn't do.
Anyways, I think the man page isn't very clear on this fact...
Agreed and I guess this is why the 'know your interfaces' maxim is so important.
While that are more robust ways to handle the case then desc is NULL (which for my glibc it never is) the above code is safe since both isempty() and glibc's free() can safely handle NULL pointers.
What about this?
if (description!=NULL && (n != 4 || isempty(description)))
Looks fine to me. For belt and braces we should probably also assert that description is NULL when we enter the call (or test it and free it).