[Webkit-unassigned] [Bug 101079] [GTK] Clean up g_return macros usage in GObject DOM bindings

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Nov 8 08:59:42 PST 2012


--- Comment #3 from Xan Lopez <xan.lopez at gmail.com>  2012-11-08 09:01:18 PST ---
(From update of attachment 172108)
View in context: https://bugs.webkit.org/attachment.cgi?id=172108&action=review

Looks mostly good to me, I just have a couple of questions.

> Source/WebCore/ChangeLog:11
> +          - Use them only to check parameters of public API.

Not trying to get into a philosophical debate here, and it's really not so relevant for this bug because I basically agree with the changes, but I don't see this as a good practice per se. The good practice about g_return macros is to use them in public API instead of ASSERTs or no checks at all. The reason for that is that we cannot control what the user gives us, so it makes sense to check things but it does not make sense to crash on error. That being said, using g_return macros internally makes sense for situations where you want to fail, but do not want to crash (ie, for situations that are neither a programmer's error nor so critical as to justify just giving up and crashing the program).

> Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm:1233
> +    return request ? request->priv->coreObject.get() : 0;

This seems consistent with what we do in most places (but not all!) in WebKitGTK+, so I think that's OK.

> Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm:1245
> +    ASSERT(coreObject);

Any reason why this should be different than ::core or ::kit?

> Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm:1346
> +        return 0;

Same comment than in ::core..

Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

More information about the webkit-unassigned mailing list