Re: Oaf leak fix ...

[Maciej Stachowiak <mjs eazel com>]

Michael Meeks <michael helixcode com> writes:

> Hi there,
> 
> 	I noticed a leak in oaf's configuration file handling
> recently; here is an untested, but trivial patch:
>

Looks good, but make sure to test it before committing.
 
> Index: ChangeLog
> ===================================================================
> RCS file: /cvs/gnome/oaf/ChangeLog,v
> retrieving revision 1.126
> diff -u -r1.126 ChangeLog
> --- ChangeLog   2000/11/16 23:08:07     1.126
> +++ ChangeLog   1997/01/01 00:41:21
> @@ -1,3 +1,7 @@
> +2000-11-24  Michael Meeks  <michael helixcode com>
> +
> +       * oafd/od-utils.c (od_utils_load_config_file): fix leak.
> +
>  2000-11-16  Elliot Lee  <sopwith redhat com>
> 
>         * (oaf_server_by_forking) Use $OAF_OD_IOR environment variable
> Index: oafd/od-utils.c
> ===================================================================
> RCS file: /cvs/gnome/oaf/oafd/od-utils.c,v
> retrieving revision 1.8
> diff -u -r1.8 od-utils.c
> --- oafd/od-utils.c     2000/11/09 17:49:59     1.8
> +++ oafd/od-utils.c     1997/01/01 00:41:22
> @@ -62,7 +62,7 @@
>          doc = od_utils_load_xml_file ();
> 
>          search_node = doc->xmlRootNode->xmlChildrenNode;
> -        result = "";
> +        result = g_strdup ("");
>          while (search_node != NULL) {
>                  if (strcmp (search_node->name, "searchpath") == 0) {
>                          xmlNodePtr item_node;
> @@ -70,11 +70,14 @@
>                          while (item_node != NULL) {
>                                  if (strcmp (item_node->name, "item") ==
> 0) {
>                                          char *directory;
> -                                        /* FIXME bugzilla.eazel.com
> 2726: this may be slow and has probably
> -                                           a direct influence on startup
> time. */
> +                                        char *old_result = result;
> +
>                                          directory = xmlNodeGetContent
> (item_node);
> -                                        result = g_strconcat (result,
> ":", directory, NULL);
> -                                        xmlFree (directory);
> +                                        if (directory) {
> +                                                result = g_strconcat
> (old_result, ":", directory, NULL);
> +                                                xmlFree (directory);
> +                                                g_free (old_result);
> +                                        }
>                                  }
>                                  item_node = item_node->next;
>                          }
> 
> 	Also; I see po/Makefile.in.in is in CVS; is this not an
> autogenerated file ?
> 

It should be removed I think. I've always been confused by that
stuff. Want to take care of it?

 - Maciej