[PATCH] location-gps-nmea: fix memory leaks

Aleksander Morgado aleksander at aleksander.es
Fri Aug 4 13:06:41 UTC 2017


>>              sequence = g_strdup_printf ("%s%s%s",
>>                                          previous,
>> @@ -118,7 +121,14 @@ gboolean
>>  mm_location_gps_nmea_add_trace (MMLocationGpsNmea *self,
>>                                  const gchar *trace)
>>  {
>> -    return location_gps_nmea_take_trace (self, g_strdup (trace));
>> +    char *trace_copy;
>> +
>> +    trace_copy = g_strdup (trace);
>> +    if (location_gps_nmea_take_trace (self, trace_copy))
>> +        return TRUE;
>> +
>> +    g_free (trace_copy);
>> +    return FALSE;
>>  }
>
> Since take_trace() is an internal function, and the current callers
> don't do anything useful with a FALSE return, maybe we should just
> update take_trace() to consume the input regardless of the return?
> Otherwise callers will have some strings consumed, and others not
> consumed, and the caller has to track that.  Seems kinda pointless.
>
>
> I was wondering the same thing, but not sure what's the typical semantic or
> convention for something called *_take_*(). I would prefer a simpler
> semantic, like you said, that it always take the ownership of the argument.
> But if there is an established semantic or convention, I would be hesitated
> to change it but that wouldn't prevent us from naming the function
> differently ;)
>

Oh, the convention for something called take_() should be that the
ownership of the variable is transferred, so totally agree that the
method should free() it internally.

-- 
Aleksander
https://aleksander.es


More information about the ModemManager-devel mailing list