Re: Memory leaks in windres

[Alan Modra]

On Fri, May 03, 2002 at 08:02:43PM +0200, Borut Razem wrote:

> FILE: rclex.l
> FUNCTION: get_string()
> LINE: 455
> OLD CODE:
>   strings = as->next;
> NEW CODE:
>   strings = as;
> DESCRIPTION:
>   Strings are incorrect linked to the list: strings variable should
>   point to the last element in the list, where new elements are appended.
>   As a consequence of the defect, the value of the strings variable is
>   always NULL and rcparse_discard_strings() function does not free
>   allocated list of strings.
> 
> --------------
> FILE: resrc.c
> FUNCTION: read_rc_file ()
> OLD CODE:
>   yyparse();
>   rcparse_discard_strings ();
> NEW CODE:
>   rcparse_discard_strings ();
> DESCRIPTION:
>   The last list of strings, allocated in rclex.c by get_string(), is
>   never deallocated.
> SOLUTION:
>   deallocate the list of strings by calling rcparse_discard_strings()
>   after yylex().

I'm applying the above two patches.  Thanks!

> --------------
> FILE: resrc.c
> FUNCTION: define_icon (id, resinfo, filename)
> LINE: 1020
> OLD CODE:
>   icondirs = (struct icondir *) xmalloc (count * sizeof *icondirs);
> NEW CODE:
>   icondirs = (struct icondir *) alloca (count * sizeof *icondirs);
> DESCRIPTION:
>   use alloca instead xmalloc, remove free (icondirs); on line 1100

This one isn't such a good idea if "count" can be large.  Allocating
large stack frames can result in errors on machines with limited
stack size.

> --------------
> FILE: resrc.c
> FUNCTION: define_control (text, id, x, y, width, height, _class, style,
> exstyle)
> LINE: 844, 845
> OLD CODE:
>   if (text == NULL)
>     text = "";
> NEW CODE:
>   //none
> DESCRIPTION:
> - define_icon_control() calls define_control() with text set to 0
> - define_control() sets the text to empty string and calls
> res_string_to_id(),
>   which allocates an id with empty unicode string
> - define_icon_control() then redefines the id:   n->text = iid;,
>   without freeing existing id (actually the empty unicode string)
> SOLUTION:
> - define_control() should not set the text to empty string in case
>   of NULL text. res_string_to_id() should be changed to handle
>   NULL string properly.
> 
> 
> --------------
> FILE: windres.c
> FUNCTION: res_string_to_id (res_id, string)
> LINE: 341
> OLD CODE:
>   unicode_from_ascii (&res_id->u.n.length, &res_id->u.n.name, string);
> NEW CODE:
>   if (string == NULL)
>     res_id->u.n.name = NULL;
>   else
>     unicode_from_ascii (&res_id->u.n.length, &res_id->u.n.name, string);
> DESCRIPTION:
>   res_string_to_id() should be changed to handle NULL string properly.
> SOLUTION:
>   res_string_to_id() accepts NULL string. In that case sets res_id->u.n.name
>   to NULL.

I'm not willing to apply the above two patches either.  Some code
somewhere may rely on the presence of u.n.name pointing to a zero
word when u.n.length is zero.  I couldn't find any such code with
a quick scan over the relevant files, but this isn't an area of
binutils I'm that familiar with.  You'll need to convince me, or
wait for another maintainer more familiar with the code to commit
it.

> I'm working on a library, with which it will be possible to directly
> load a resource from an ASCII source file into application, without
> the need to (re)compile and (re)link it. I wold like to publish the
> library under LGPL (and not GPL) license, but the library will include
> rcparse.l, rclex.l, resbin.c and resrc.c files from binutils.
> Is there a possibility to change the license of mentioned files
> from GPL to LGPL?

Ask address@hidden

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre