[Spice-devel] [PATCH spice-gtk 2/3] usb-device-manager: Don't show an error dialog on a cancelled device open

[Hans de Goede]

Hi,

On 11/21/2011 03:04 PM, Marc-André Lureau wrote:
>
> Hi
>
>>>> +    if (err)
>>>> +        g_error_free(err);
>>>
>>> In general, we should favor g_clear_error(), which is also NULL
>>> tolerant.
>>
>> Hmm, I checked the docs before doing things this way and the docs
>> say:
>>
>> void                g_clear_error                       (GError
>> **err);
>>
>> If err is NULL, does nothing. If err is non-NULL, calls
>> g_error_free() on *err and sets *err to NULL.
>>
>> So this means that **err may by NULL, but *err will get passed to
>> g_error_free(), which is not NULL tolerant (I did check that).
>>
>> I also just checked the actual code for g_clear_error and the
>> documentation
>> is correct, so using g_clear_error(&err) instead of:
>>       if (err)
>>           g_error_free(err);
>> Will cause g_error_free to get called with a NULL, which it does not
>> like.
>
> http://git.gnome.org/browse/glib/tree/glib/gerror.c#n623
>
> void
> g_clear_error (GError **err)
> {
>    if (err&&  *err)
>      {
>        g_error_free (*err);
>        *err = NULL;
>      }
> }
>
> I don't see how g_error_free() can be called with NULL.

Oh right, I was in a hurry and misread the code, sorry. Ok so I will
1) fix the patch to use g_clear_error in my next revision of the patchset

2) File a bug against glib asking them to clearly document that g_clear_error
is NULL safe both for err and for *err.

Regards,

Hans

p.s.

Any reaction to my comments on your review of the first patch? I would like
to get that patch into an acceptable shape too before resubmitting the
patchset for another review.