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

[Aleksander Morgado]

On 03/08/17 23:49, Ben Chan wrote:
> There are potential memory leaks in MMLocationGpsNmea:
> 
> - When the `trace' string provided to location_gps_nmea_take_trace() isn't
>   added to the hash table, its ownership is still considered transferred.
>   It should thus be freed. Similarly, the `trace_type' string isn't
>   added the hash table and should thus be freed.
> 
> - mm_location_gps_nmea_add_trace() duplicates a given trace string and
>   then passes the trace copy to location_gps_nmea_take_trace(). When
>   location_gps_nmea_take_trace() returns FALSE, the ownership of the
>   copy isn't transferred. mm_location_gps_nmea_add_trace() should thus
>   free the copy.
> 
> This patch fixes the above memory leaks by having
> location_gps_nmea_take_trace() always take the ownership of the `trace'
> string and internally free `trace' and `trace_type' when necessary.
> ---

Pushed to git master, mm-1-6, mm-1-4 and mm-1-2, thanks!

>  libmm-glib/mm-location-gps-nmea.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/libmm-glib/mm-location-gps-nmea.c b/libmm-glib/mm-location-gps-nmea.c
> index 08ec97ff..9b4f9daf 100644
> --- a/libmm-glib/mm-location-gps-nmea.c
> +++ b/libmm-glib/mm-location-gps-nmea.c
> @@ -77,8 +77,10 @@ location_gps_nmea_take_trace (MMLocationGpsNmea *self,
>      gchar *trace_type;
>  
>      i = strchr (trace, ',');
> -    if (!i || i == trace)
> +    if (!i || i == trace) {
> +        g_free (trace);
>          return FALSE;
> +    }
>  
>      trace_type = g_malloc (i - trace + 1);
>      memcpy (trace_type, trace, i - trace);
> @@ -96,8 +98,11 @@ location_gps_nmea_take_trace (MMLocationGpsNmea *self,
>              gchar *sequence;
>  
>              /* Skip the trace if we already have it there */
> -            if (strstr (previous, trace))
> +            if (strstr (previous, trace)) {
> +                g_free (trace_type);
> +                g_free (trace);
>                  return TRUE;
> +            }
>  
>              sequence = g_strdup_printf ("%s%s%s",
>                                          previous,
> @@ -222,8 +227,7 @@ mm_location_gps_nmea_new_from_string_variant (GVariant *string,
>      self = mm_location_gps_nmea_new ();
>  
>      for (i = 0; split[i]; i++) {
> -        if (!location_gps_nmea_take_trace (self, split[i]))
> -            g_free (split[i]);
> +        location_gps_nmea_take_trace (self, split[i]);
>      }
>  
>      /* Note that the strings in the array of strings were already taken
> 


-- 
Aleksander
https://aleksander.es