memleak in FScreenParseGeometryWithScreen

[Dominik Vogt]

From FScreen.c:FScreenParseGeometryWithScreen():

943     copy = fxstrdup(parsestring);
944     copy = strsep(&parsestring, "@");
945
946     *screen_return = fxstrdup(parsestring);
947     geom_str = strsep(&copy, "@");
948     copy = geom_str;

I can't fix this because I don't understand what the code does.

Line 943 allocates some memory and stores the pointer in "copy.        OK

Line 944 throws away the just allocated "copy"                     LEAK A
         It tokenizes the "parsestring" passed in by the caller,
         thus destroying its value.                                   BUG
         The code seems to assume that after "copy = strsep()" the
         copy pointer would point to the _next_ token?  But it
         will be identical to "parsestring".                         BUG?

Line 946 Returns a copy of the token = the geometry string without
         the screen portion through *screen_return.  There are two
         bugs here:

         1) The callers expect the token with the screen string,
            not the token with the geometry string.                   BUG

         2) The callers don't know that they have to free() the
            returned pointer.                                      LEAK B

Line 947 tokenizes the already tokenized token again, which           BUG
         is a no-op.

Line 948 overwrites "copy" with its own value => no-op.               BUG

--

Summary:

1) LEAK A is inside the function and can be fixed there.
2) LEAK B is in the callers.  None of the callers I checked is aware
   that *screen_return has to be free()'d.
3) *screen_return contains the geometry portion of the string, but
   callers expect it to contain the screen portion.

Ciao

Dominik ^_^  ^_^

--

Dominik Vogt