bug-gnu-utils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Memory leaks in windres


From: Borut Razem
Subject: Memory leaks in windres
Date: Fri, 3 May 2002 20:02:43 +0200

Hello,

I found some defects (memory leaks) and possible enhancements (replace
xmalloc with alloca):


--------------
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().


--------------
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


--------------
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.


And here is the patch:

===================================================================
diff -c -3 -p -w -r latest/rclex.l changed/rclex.l
*** latest/rclex.l      Fri May  3 09:02:33 2002
--- changed/rclex.l     Fri May  3 18:02:09 2002
*************** get_string (len)
*** 452,458 ****
    as->s = xmalloc (len);

    as->next = strings;
!   strings = as->next;

    return as->s;
  }
--- 452,458 ----
    as->s = xmalloc (len);

    as->next = strings;
!   strings = as;

    return as->s;
  }
diff -c -3 -p -w -r latest/resrc.c changed/resrc.c
*** latest/resrc.c      Fri May  3 09:06:14 2002
--- changed/resrc.c     Fri May  3 19:01:48 2002
*************** read_rc_file (filename, preprocessor, pr
*** 481,486 ****
--- 481,487 ----
      rcparse_set_language (language);
    yyin = cpp_pipe;
    yyparse ();
+   rcparse_discard_strings ();

    close_input_stream ();

*************** define_control (text, id, x, y, width, h
*** 841,848 ****
    n->height = height;
    n->class.named = 0;
    n->class.u.id = class;
-   if (text == NULL)
-     text = "";
    res_string_to_id (&n->text, text);
    n->data = NULL;
    n->help = 0;
--- 842,847 ----
*************** define_icon (id, resinfo, filename)
*** 1017,1023 ****

    /* Read in the icon directory entries.  */

!   icondirs = (struct icondir *) xmalloc (count * sizeof *icondirs);

    for (i = 0; i < count; i++)
      {
--- 1016,1022 ----

    /* Read in the icon directory entries.  */

!   icondirs = (struct icondir *) alloca (count * sizeof *icondirs);

    for (i = 0; i < count; i++)
      {
*************** define_icon (id, resinfo, filename)
*** 1097,1104 ****
        pp = &(*pp)->next;
      }

-   free (icondirs);
-
    r = define_standard_resource (&resources, RT_GROUP_ICON, id,
                                resinfo->language, 0);
    r->type = RES_TYPE_GROUP_ICON;
--- 1096,1101 ----
diff -c -3 -p -w -r latest/windres.c changed/windres.c
*** latest/windres.c    Fri May  3 12:54:29 2002
--- changed/windres.c   Fri May  3 12:52:53 2002
*************** res_string_to_id (res_id, string)
*** 338,343 ****
--- 338,346 ----
       const char *string;
  {
    res_id->named = 1;
+   if (string == NULL)
+     res_id->u.n.name = NULL;
+   else
      unicode_from_ascii (&res_id->u.n.length, &res_id->u.n.name, string);
  }

===================================================================

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?
I would also like to split (or maybe #ifdef) resbin.c and resrc.c
to two pats:
bin_to_res and res_to_bin, define_... and write_..., because
only one part of each file will be used by the library. Can I do that?


Borut Razem




reply via email to

[Prev in Thread] Current Thread [Next in Thread]