Bug#366253: Debian bug followup (Re: bug 366253)

[Loïc Minier]

tags 366253 - patch
stop

        Hi,

On Wed, Oct 17, 2007, Emil Nowak wrote:
> When I read it now It seems to obvious to ask on mailing list. 
> If you pass NULL value as "parent" variable to this expression:
> g_return_if_fail (parent == NULL);
> if will evaluate to
> g_return_if_fail (NULL == NULL); 
> which is TRUE so it never fails. IMHO someone just made a typo here. Attached
> patch fixes that.

 Err I think it's a feature; the current API doesn't document what
 happens when you pass parent == NULL to this func.  I guess it's simply
 easier to cope and not complain if parent is NULL.  When I read:

    g_return_if_fail (parent == NULL || GTK_IS_WINDOW (parent));

 I understand that it's ok for parent to be NULL, but if it's set, then
 it should be a valid GtkWindow.

 So IMO this isn't a bug but a feature: Gtk+ simply accepts the parent
 == NULL use case.

 BTW your proposed patch is incorrect, it wouldn't ever check for
 GTK_IS_WINDOW (parent); you proposed:

    g_return_if_fail (parent != NULL || GTK_IS_WINDOW (parent));

 but this would fail twice with parent == NULL, and GTK_IS_WINDOW
 already checks for NULLity anyway.  What you meant was probably:

    g_return_if_fail (parent != NULL && GTK_IS_WINDOW (parent));

 which is equivalent to g_return_if_fail(GTK_IS_WINDOW (parent));


 I'm tempted to close this bug, but please shout if this is incorrect.

-- 
Loïc Minier