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?
Anyways, I think the man page isn't very clear on this fact...
Thus the simpler fix for this leak, and one that is not prone to buffer overflow, is:
--- vdr-1.3.22/epg.c.orig 2005-03-29 09:03:23.484024000 +0100 +++ vdr-1.3.22/epg.c 2005-03-29 09:04:16.359024000 +0100 @@ -29,9 +29,7 @@ { unsigned int Stream, Type; int n = sscanf(s, "%X %02X %3c %a[^\n]", &Stream, &Type, language, &description);
- if (n != 4)
description = NULL;- else if (isempty(description)) {
- if (n != 4 || isempty(description)) { free(description); description = NULL; }
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)))
Regards.