[PATCH] tests: fix GError pointer initialization and memory leaks

[Ben Chan]

Gotcha. I guess the GError* and gchar* leaks are probably ok if the program
aborts.


On Tue, Jun 3, 2014 at 1:56 AM, Aleksander Morgado <aleksander at aleksander.es
> wrote:

> On 03/06/14 04:10, Ben Chan wrote:
> > This patch fixes a few memory leaks and clears a few GError pointers
> > before reusing them.
> > ---
>
>
> g_error() explicitly aborts the program; so there really is no point in
> clearing the error afterwards because the program will no longer be
> running:
>
> https://developer.gnome.org/glib/stable/glib-Message-Logging.html#g-error
>
>
> >  plugins/tests/test-fixture.c      | 20 +++++++++++++++-----
> >  plugins/tests/test-port-context.c | 29 +++++++++++++++++++++--------
> >  2 files changed, 36 insertions(+), 13 deletions(-)
> >
> > diff --git a/plugins/tests/test-fixture.c b/plugins/tests/test-fixture.c
> > index 6b10f47..2fe1d59 100644
> > --- a/plugins/tests/test-fixture.c
> > +++ b/plugins/tests/test-fixture.c
> > @@ -34,8 +34,10 @@ test_fixture_setup (TestFixture *fixture)
> >
> >      /* Create DBus connection */
> >      fixture->connection = g_bus_get_sync (G_BUS_TYPE_SESSION, NULL,
> &error);
> > -    if (fixture->connection == NULL)
> > +    if (fixture->connection == NULL) {
> >          g_error ("Error getting connection to test bus: %s",
> error->message);
> > +        g_clear_error (&error);
> > +    }
> >
> >      /* Ping to autostart MM; wait up to 3s */
> >      result = g_dbus_connection_call_sync (fixture->connection,
> > @@ -49,8 +51,10 @@ test_fixture_setup (TestFixture *fixture)
> >                                            3000, /* timeout, ms */
> >                                            NULL, /* cancellable */
> >                                            &error);
> > -    if (!result)
> > +    if (!result) {
> >          g_error ("Error starting ModemManager in test bus: %s",
> error->message);
> > +        g_clear_error (&error);
> > +    }
> >      g_variant_unref (result);
> >
> >      /* Create the proxy that we're going to test */
> > @@ -60,8 +64,10 @@ test_fixture_setup (TestFixture *fixture)
> >
>  "/org/freedesktop/ModemManager1",
> >                                                    NULL, /* cancellable
> */
> >                                                    &error);
> > -    if (fixture->test == NULL)
> > +    if (fixture->test == NULL) {
> >          g_error ("Error getting ModemManager test proxy: %s",
> error->message);
> > +        g_clear_error (&error);
> > +    }
> >  }
> >
> >  void
> > @@ -111,8 +117,10 @@ test_fixture_get_modem (TestFixture *fixture)
> >
> G_DBUS_OBJECT_MANAGER_CLIENT_FLAGS_NONE,
> >                                     NULL, /* cancellable */
> >                                     &error);
> > -    if (!manager)
> > +    if (!manager) {
> >          g_error ("Couldn't create manager: %s", error->message);
> > +        g_clear_error (&error);
> > +    }
> >
> >      /* Find new modem object */
> >      while (!found) {
> > @@ -157,8 +165,10 @@ test_fixture_no_modem (TestFixture *fixture)
> >
> G_DBUS_OBJECT_MANAGER_CLIENT_FLAGS_NONE,
> >                                     NULL, /* cancellable */
> >                                     &error);
> > -    if (!manager)
> > +    if (!manager) {
> >          g_error ("Couldn't create manager: %s", error->message);
> > +        g_clear_error (&error);
> > +    }
> >
> >      /* Find new modem object */
> >      while (!no_modems) {
> > diff --git a/plugins/tests/test-port-context.c
> b/plugins/tests/test-port-context.c
> > index cbf202f..53c6ef3 100644
> > --- a/plugins/tests/test-port-context.c
> > +++ b/plugins/tests/test-port-context.c
> > @@ -53,10 +53,16 @@ test_port_context_load_commands (TestPortContext
> *self,
> >      gchar *contents;
> >      gchar *current;
> >
> > -    if (!g_file_get_contents (file, &contents, NULL, &error))
> > +    if (!g_file_get_contents (file, &contents, NULL, &error)) {
> > +        gchar *filename;
> > +
> > +        filename = g_filename_display_name (file);
> >          g_error ("Couldn't load commands file '%s': %s",
> > -                 g_filename_display_name (file),
> > +                 filename,
> >                   error->message);
> > +        g_free (filename);
> > +        g_clear_error (&error);
> > +    }
> >
> >      current = contents;
> >      while (current) {
> > @@ -198,8 +204,7 @@ connection_readable_cb (GSocket *socket,
> >
> >      if (r < 0) {
> >          g_warning ("Error reading from istream: %s", error ?
> error->message : "unknown");
> > -        if (error)
> > -            g_error_free (error);
> > +        g_clear_error (&error);
> >          /* Close the device */
> >          connection_close (client);
> >          return FALSE;
> > @@ -269,20 +274,26 @@ create_socket_service (TestPortContext *self)
> >                             G_SOCKET_TYPE_STREAM,
> >                             G_SOCKET_PROTOCOL_DEFAULT,
> >                             &error);
> > -    if (!socket)
> > +    if (!socket) {
> >          g_error ("Cannot create socket: %s", error->message);
> > +        g_clear_error (&error);
> > +    }
> >
> >      /* Bind to address */
> >      address = (g_unix_socket_address_new_with_type (
> >                     self->name,
> >                     -1,
> >                     G_UNIX_SOCKET_ADDRESS_ABSTRACT));
> > -    if (!g_socket_bind (socket, address, TRUE, &error))
> > +    if (!g_socket_bind (socket, address, TRUE, &error)) {
> >          g_error ("Cannot bind socket: %s", error->message);
> > +        g_clear_error (&error);
> > +    }
> >
> >      /* Listen */
> > -    if (!g_socket_listen (socket, &error))
> > +    if (!g_socket_listen (socket, &error)) {
> >          g_error ("Cannot listen in socket: %s", error->message);
> > +        g_clear_error (&error);
> > +    }
> >
> >      /* Create socket service */
> >      service = g_socket_service_new ();
> > @@ -290,8 +301,10 @@ create_socket_service (TestPortContext *self)
> >      if (!g_socket_listener_add_socket (G_SOCKET_LISTENER (service),
> >                                         socket,
> >                                         NULL, /* don't pass an object,
> will take a reference */
> > -                                       &error))
> > +                                       &error)) {
> >          g_error ("Cannot add listener to socket: %s", error->message);
> > +        g_clear_error (&error);
> > +    }
> >
> >      /* Start it */
> >      g_socket_service_start (service);
> >
>
>
> --
> Aleksander
> https://aleksander.es
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/modemmanager-devel/attachments/20140603/5976a8c8/attachment-0001.html>