[Spice-devel] [PATCH GTK 1/4] more simplification of spice_smartcard_manager_init_libcacard

[Marc-André Lureau]

Hi,

We don't see it in the diff, but the resulting code may leak here:

    g_object_get(G_OBJECT(session),
                 "smartcard-db", &dbname,
                 "smartcard-certificates", &certificates,
                 NULL);

    if ((certificates == NULL) || (g_strv_length(certificates) != 3))
        goto init;

Either dbname, certificates or both.

I prefer having the free of everything in the "end". It's much easier
to read when there is multiple paths.

Btw, shouldn't we print a warning/error if g_strv_length(certificates) != 3 ?

regards
On Tue, Jul 5, 2011 at 10:37 AM, Christophe Fergeau <cfergeau at redhat.com> wrote:
> By freeing memory as soon as it's not longer used, we can
> remove the end: label and the retval variable.
> ---
>  gtk/smartcard-manager.c |   14 +++++---------
>  1 files changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/gtk/smartcard-manager.c b/gtk/smartcard-manager.c
> index 86c5509..0aeb4f1 100644
> --- a/gtk/smartcard-manager.c
> +++ b/gtk/smartcard-manager.c
> @@ -396,7 +396,6 @@ gboolean spice_smartcard_manager_init_libcacard(SpiceSession *session)
>     VCardEmulOptions *options = NULL;
>     gchar *dbname = NULL;
>     GStrv certificates;
> -    gboolean retval = FALSE;
>
>     g_return_val_if_fail(session != NULL, VCARD_EMUL_FAIL);
>     g_object_get(G_OBJECT(session),
> @@ -413,28 +412,25 @@ gboolean spice_smartcard_manager_init_libcacard(SpiceSession *session)
>                                     dbname, SPICE_SOFTWARE_READER_NAME,
>                                     certificates[0], certificates[1],
>                                     certificates[2]);
> +        g_free(dbname);
>     } else {
>         emul_args = g_strdup_printf("use_hw=no soft=(,%s,CAC,,%s,%s,%s)",
>                                     SPICE_SOFTWARE_READER_NAME,
>                                     certificates[0], certificates[1],
>                                     certificates[2]);
>     }
> +    g_strfreev(certificates);
>
>     options = vcard_emul_options(emul_args);
> +    g_free(emul_args);
>     if (options == NULL) {
>         g_critical("vcard_emul_options() failed!");
> -        goto end;
> +        return FALSE;
>     }
>
>  init:
>     /* FIXME: sometime this hangs a long time..., cant we make it async? */
> -    retval = vcard_emul_init(options) == VCARD_EMUL_OK;
> -
> -end:
> -    g_free(emul_args);
> -    g_free(dbname);
> -    g_strfreev(certificates);
> -    return retval;
> +    return vcard_emul_init(options) == VCARD_EMUL_OK;
>  }
>
>  gboolean spice_smartcard_manager_insert_card(SpiceSmartCardManager *manager)
> --
> 1.7.5.4
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>



-- 
Marc-André Lureau