<div dir="ltr">Gotcha. I guess the GError* and gchar* leaks are probably ok if the program aborts.</div><div class="gmail_extra"> <div class="gmail_quote">On Tue, Jun 3, 2014 at 1:56 AM, Aleksander Morgado <<a href="mailto:aleksander@aleksander.es" target="_blank">aleksander@aleksander.es</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="">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. > --- </div>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: <a href="https://developer.gnome.org/glib/stable/glib-Message-Logging.html#g-error" target="_blank">https://developer.gnome.org/glib/stable/glib-Message-Logging.html#g-error</a> <div><div class="h5"> > 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); > -- </div></div>Aleksander <a href="https://aleksander.es" target="_blank">https://aleksander.es</a> </blockquote></div> </div>