Re: [Qemu-trivial] [Qemu-devel] [PATCH 4/4] vl.c: fix memory leak

[Leon Alrae]

On 10/07/2015 01:51, address@hidden wrote:
> From: Gonglei <address@hidden>
> 
> Failing to save or free storage allocated
> by "g_strdup(cmd)" leaks it. Let's use a
> variable to storage it.
> 
> Signed-off-by: Gonglei <address@hidden>
> ---
>  vl.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/vl.c b/vl.c
> index 3f269dc..399e816 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1326,16 +1326,19 @@ static int add_semihosting_arg(void *opaque,
>  static inline void semihosting_arg_fallback(const char *file, const char 
> *cmd)
>  {
>      char *cmd_token;
> +    char *cmd_str;
>  
>      /* argv[0] */
>      add_semihosting_arg(&semihosting, "arg", file, NULL);
>  
> +    cmd_str = g_strdup(cmd);
>      /* split -append and initialize argv[1..n] */
> -    cmd_token = strtok(g_strdup(cmd), " ");
> +    cmd_token = strtok(cmd_str, " ");
>      while (cmd_token) {
>          add_semihosting_arg(&semihosting, "arg", cmd_token, NULL);
>          cmd_token = strtok(NULL, " ");
>      }
> +    g_free(cmd_str);
>  }
>  
>  /***********************************************************/
> 

I don't think this is correct as there's no leak here. This duplicated string
is modified (i.e. split into tokens) and each pointer to the beginning of a
token is saved in the global semihosting.argv[] array which is used later in
target semihosting code. It shouldn't be freed.

Regards,
Leon