Stefan Huelswitt wrote:
On 27 Mar 2005 Clemens Kirchgatterer clemens@1541.org wrote:
s.huelswitt@gmx.de (Stefan Huelswitt) wrote:
Second is in epg.c tComponent::FromString(). I cannot find anything bad with the code there, but valgrind reports a lot of memory leaks with the sscanf() call. So I guessed that sscanf() is leaking internaly when used with "%a[\n]" (at least with my glibc version 2.2.5). After changing to code to the suggestion below, the leaks disappeared:
from man sscanf:
a Indicates that the conversion will be s, the needed memory space for the string will be malloc'ed and the pointer to it will be assigned to the char pointer variable, which does not have to be initialized before.
so, yes, the user is responsible for freeing memory allocated by sscanf.
Yes, of course. I can read man pages ;)
This is not what I'm talking about. The malloc memory is free'd by VDR, but there is still some memory leaked. I think it's internaly lost. This can be proven with my patch. If the %a[ is obmitted, there is no leak.
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.
n = sscanf("", "%a[^\n]", &desc); printf("n = %d; desc = %p\n", n, desc);
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.
Regards.