[PATCH v2 12/25] [libx11] Instead of copying the value returned by get_prop_name and then releasing it, directly use the return value of get_prop_name, which allocates memory for the name.

Alan Coopersmith alan.coopersmith at oracle.com
Mon Jan 31 23:42:26 PST 2011


On 01/31/11 04:02 AM, Erkki Seppälä wrote:
> Subject: [PATCH v2 12/25] [libx11] Instead of copying the value returned by
get_prop_name and then releasing it, directly use the return value of
get_prop_name, which allocates memory for the name.
> Variable "prop_name" not freed or pointed-to in function "strlen"

The shortlog/summary/subject line of the patch really should fit into about 72
characters so it's readable in git shortlog output in a 80 column terminal.

> Reviewed-by: Ander Conselvan de Oliveira <ander.conselvan-de-oliveira at nokia.com>
> Signed-off-by: Erkki Seppälä <erkki.seppala at vincit.fi>
> ---
>  src/xlibi18n/XDefaultOMIF.c |    4 +---
>  1 files changed, 1 insertions(+), 3 deletions(-)
> 
> diff --git a/src/xlibi18n/XDefaultOMIF.c b/src/xlibi18n/XDefaultOMIF.c
> index bb3986a..f4f141c 100644
> --- a/src/xlibi18n/XDefaultOMIF.c
> +++ b/src/xlibi18n/XDefaultOMIF.c
> @@ -403,9 +403,7 @@ get_font_name(
>  	    return NULL;
>          }
>  
> -	name = (char*) Xmalloc(strlen(prop_name) + 1);
> -	if (name)
> -	    strcpy(name, prop_name);
> +	name = prop_name;
>  
>  	XFreeFont(dpy, fs);
>      }

So after looking at this, I think you should drop the previous [PATCH v2 01/25]
and solve both problems in one change, with even less code:

XDefaultOMIF.c: Fix memory leaks in get_font_name

Instead of copying the value returned by get_prop_name and then releasing it,
directly use the return value of get_prop_name, which allocates memory for the
name.

If get_prop_name returns NULL, continue on to XFreeFont to release the font
before returning the NULL via the normal function return.

--- a/src/xlibi18n/XDefaultOMIF.c
+++ b/src/xlibi18n/XDefaultOMIF.c
@@ -381,7 +381,7 @@ get_font_name(
     XOC oc,
     char *pattern)
 {
-    char **list, *name, *prop_name;
+    char **list, *name;
     int count;
     XFontStruct *fs;
     Display *dpy = oc->core.om->core.display;
@@ -397,13 +397,7 @@ get_font_name(
        fs = XLoadQueryFont(dpy, pattern);
        if (fs == NULL) return NULL;

-       prop_name = get_prop_name(dpy, fs);
-       if (prop_name == NULL) return NULL;
-
-       name = (char*) Xmalloc(strlen(prop_name) + 1);
-       if (name)
-           strcpy(name, prop_name);
-
+       name = get_prop_name(dpy, fs);
        XFreeFont(dpy, fs);
     }
     return name;


Plus this would solve the "unused variable prop_name" warning that your patch
would introduce.

-- 
	-Alan Coopersmith-        alan.coopersmith at oracle.com
	 Oracle Solaris Platform Engineering: X Window System



More information about the xorg-devel mailing list