[PATCH] huawei plugin: fix memory leaks in tests.

[Aleksander Morgado]

On Wed, Mar 25, 2015 at 5:30 PM, Yunlian Jiang <yunlian at chromium.org> wrote:
> ---
>  plugins/huawei/tests/test-modem-helpers-huawei.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/plugins/huawei/tests/test-modem-helpers-huawei.c
> b/plugins/huawei/tests/test-modem-helpers-huawei.c
> index 9e92eee..ef1118a 100644
> --- a/plugins/huawei/tests/test-modem-helpers-huawei.c
> +++ b/plugins/huawei/tests/test-modem-helpers-huawei.c
> @@ -1087,6 +1087,9 @@ test_nwtime (void)
>
>          if (iso8601)
>              g_free (iso8601);

I know this is not part of your patch, but there's no need to check
for NULL before doing g_free(). I'll gladly accept a patch revising
this whole file and changing those.

> +
> +       if (tz)
> +           g_free (tz);

tz is actually a MMNetworkTimezone, which is a GObject. The proper fix would be:
   if (tz)
      g_object_unref (tz);

>      }
>  }
>
> @@ -1128,6 +1131,8 @@ test_time (void)
>
>          g_assert_cmpstr (time_tests[i].iso8601, ==, iso8601);
>
> +       g_clear_error (&error);
> +

Looks like the test never checks if error is set.

I'd update it so that:

   if (ret)
      g_assert_no_error (error);
   else {
      g_assert_error (error, ....);
      g_error_free (error);
   }

Could you also review the remaining tests to check if error is being
checked or not?


>          if (iso8601)
>              g_free (iso8601);

Same as above, this wasn't part of your patch, but I would gladly
accept a patch changing that to just do a g_free().

>      }
> --
> 2.2.0.rc0.207.ga3a616c
>
>
> _______________________________________________
> ModemManager-devel mailing list
> ModemManager-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
>



-- 
Aleksander
https://aleksander.es