Bug 72138

You should use g_clear_error() instead. I'm actually tempted to do a big s/g_error_free/g_clear_error/ and ban g_error_free() in our coding style checker, like we already banned some _free() in favor of their corresponding _unref().

(In reply to comment #1)
> You should use g_clear_error() instead

I agree...

> I'm actually tempted to do a big s/g_error_free/g_clear_error/

... but I think this is going too far - it's often obvious that the error is both non-NULL and no longer used, for instance in this idiom for dealing with errors that there's no way to signal (yes that's a design flaw, but often one that we're stuck with medium-term):

static void
foo (void)
{
  GError *error = NULL;

  if (do_a_thing (&error))
    {
      WARNING ("%s", error->message);
      g_error_free (error);
    }
}

or when interacting with dbus-glib:

static void
do_something_on_dbus (DBusGMethodInvocation *context)
{
  GError *error = NULL;

  if (do_something (&error))
    {
      dbus_g_method_return (context);
    }
  else
    {
      dbus_g_method_return_error (context, error);
      g_error_free (error);
    }
}

Created attachment 90260 [details] [review]
new version using g_clear_error

ok, updated to use g_clear_error, I've only done that specific change and not tried to replace other uses

Comment on attachment 90260 [details] [review]
new version using g_clear_error

Review of attachment 90260 [details] [review]:
-----------------------------------------------------------------

Looks good to me.

Comment on attachment 90260 [details] [review]
new version using g_clear_error

Review of attachment 90260 [details] [review]:
-----------------------------------------------------------------

Actually, I take that back. This is g_clear_error (error), it needs to be g_clear_error (&error). Did you test this?

Fixed in git for 5.16.1; I added the necessary "&" to make it work.

ups, sorry about that, I test built but didn't test run in the buggy condition ... thanks for catching the error!