[PATCH] mbimcli: use g_strfreev instead of g_free to free g_strsplit results

[Aleksander Morgado]

Hey Ben,

On Mon, Feb 24, 2014 at 6:27 AM, Ben Chan <benchan at chromium.org> wrote:
> ---

That's wrong actually. g_strsplit () does create an array of strings
in heap, but we're returning some of the contents of the array by
reference. E.g. in the first chunk split[0] is returned as 'pin' and
split[1] is returned as 'new_pin'. The problem here is that not every
item in the split array may be returned, and that is why you may be
seen memleaks. So, either we g_strdup() the returned strings and
g_strfreev() the whole array afterwards, or we make sure the
non-returned strings are g_free()-ed independently and leave the last
g_free() for the array itself. See more comments below.


>  src/mbimcli/mbimcli-basic-connect.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/mbimcli/mbimcli-basic-connect.c b/src/mbimcli/mbimcli-basic-connect.c
> index 97368aa..9208d71 100644
> --- a/src/mbimcli/mbimcli-basic-connect.c
> +++ b/src/mbimcli/mbimcli-basic-connect.c
> @@ -642,7 +642,7 @@ set_pin_input_parse (guint         n_expected,
>      *pin = split[0];
>      *new_pin = split[1] ? split[1] : NULL;
>
> -    g_free (split);
> +    g_strfreev (split);

There shouldn't be any leak here, both split[0] and split[1] get
returned, and we end up freeing the split array (not its contents).

>      return TRUE;
>  }
>
> @@ -784,7 +784,7 @@ set_connect_activate_parse (const gchar       *str,
>          }
>      }
>
> -    g_free (split);
> +    g_strfreev (split);

In this case, I believe that only split[1] is being leaked. A simple
g_free(split[1]) would be enough, right?

>      return TRUE;
>  }
>
> --
> 1.9.0.rc1.175.g0b1dcb5
>



-- 
Aleksander
https://aleksander.es