[PATCH] libmm-glib: set clear func by default in the MMModemPortInfo array

[Piotr Figiel]

Hi Aleksander,
 I tested your patch and the reported issue doesn't appear any longer.
 It all looks good and certainly better (more glib-style) than my initial
implementation. Although I wonder if this doesn't ever collide with
mm_modem_port_info_array_free() in mm-helper-types.c? It seems that two
ways of dealing with memory management are taken for the same data
structure, though I didn't observe any double-free's on this part during
runtime, so I guess this must be correct.

Thanks for looking at this,
Piotr.


2017-02-08 18:45 GMT+01:00 Aleksander Morgado <aleksander at aleksander.es>:

> So that we don't leak the port names allocated within each
> MMModemPortInfo.
>
>     ==261== 672 bytes in 84 blocks are definitely lost in loss record
> 7,314 of 7,383
>     ==261== at 0x402C51E: malloc (vg_replace_malloc.c:299)
>     ==261== by 0x4484878: g_malloc (gmem.c:94)
>     ==261== by 0x449D51D: g_strdup (gstrfuncs.c:363)
>     ==261== by 0x44B5B73: g_variant_dup_string (gvariant.c:1529)
>     ==261== by 0x44B945E: g_variant_valist_get_nnp (gvariant.c:4775)
>     ==261== by 0x44B945E: g_variant_valist_get_leaf (gvariant.c:4945)
>     ==261== by 0x44B945E: g_variant_valist_get (gvariant.c:5126)
>     ==261== by 0x44B922C: g_variant_valist_get (gvariant.c:5161)
>     ==261== by 0x44B9FC9: g_variant_get_va (gvariant.c:5388)
>     ==261== by 0x44BA3C5: g_variant_get_child (gvariant.c:5486)
>     ==261== by 0x6613FA8: mm_common_ports_variant_to_garray
> (mm-common-helpers.c:238)
>     ==261== by 0x6601AB9: ensure_internal_ports (mm-modem.c:766)
>     ==261== by 0x6603E42: mm_modem_peek_ports (mm-modem.c:825)
>     ==261== by 0x65CD94D: owns_port (nm-modem-broadband.c:196)
>
> Reported-by: Piotr Figiel <p.figiel at camlintechnologies.com>
> ---
>
> Piotr,
>
> Could you test this patch instead of the one you provided? In this way we
> make sure that every g_array_unref() call does the proper disposal of the
> inner strings, not just during finalize().
>
> ---
>  libmm-glib/mm-common-helpers.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/libmm-glib/mm-common-helpers.c b/libmm-glib/mm-common-
> helpers.c
> index 7bd845b9..f99e58d8 100644
> --- a/libmm-glib/mm-common-helpers.c
> +++ b/libmm-glib/mm-common-helpers.c
> @@ -219,6 +219,12 @@ mm_common_sms_storages_garray_to_variant (GArray
> *array)
>      return mm_common_sms_storages_array_to_variant (NULL, 0);
>  }
>
> +static void
> +clear_modem_port_info (MMModemPortInfo *info)
> +{
> +    g_free (info->name);
> +}
> +
>  GArray *
>  mm_common_ports_variant_to_garray (GVariant *variant)
>  {
> @@ -232,6 +238,7 @@ mm_common_ports_variant_to_garray (GVariant *variant)
>
>          if (n > 0) {
>              array = g_array_sized_new (FALSE, FALSE, sizeof
> (MMModemPortInfo), n);
> +            g_array_set_clear_func (array, (GDestroyNotify)
> clear_modem_port_info);
>              for (i = 0; i < n; i++) {
>                  MMModemPortInfo info;
>
> --
> 2.11.1
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/modemmanager-devel/attachments/20170209/83fbbc17/attachment.html>