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

[Gonglei]

On 2015/7/10 17:28, Leon Alrae wrote:
> 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.
> 
Yep, I look over the logic, and you are right. :)
False positive report. NACK please.

Thanks,
-Gonglei