[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