After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 779183 - g_io_extension_point_get_extensions should check for NULL pointer
g_io_extension_point_get_extensions should check for NULL pointer
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: gio
2.51.x
Other Linux
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2017-02-24 15:46 UTC by Jan Tojnar
Modified: 2017-02-27 14:44 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gio: Check for NUll when getting extensions (937 bytes, patch)
2017-02-24 15:52 UTC, Jan Tojnar
committed Details | Review

Description Jan Tojnar 2017-02-24 15:46:27 UTC
When NULL is passed to g_io_extension_point_get_extensions, it causes a segfault.
Comment 1 Jan Tojnar 2017-02-24 15:52:19 UTC
Created attachment 346649 [details] [review]
gio: Check for NUll when getting extensions

When unregistered extension point (i.e. NULL pointer) is passed
to `g_io_extension_point_get_extensions`, it causes a segfault.

This commit adds an assertion, to prevent this.
Comment 2 Jan Tojnar 2017-02-24 15:53:43 UTC
The missing assertion manifested in https://bugzilla.gnome.org/show_bug.cgi?id=779178.
Comment 3 Carlos Soriano 2017-02-24 16:02:05 UTC
Review of attachment 346649 [details] [review]:

This looks obviously right to me, but let's wait glib maintainers.
Comment 4 Philip Withnall 2017-02-24 17:40:21 UTC
Review of attachment 346649 [details] [review]:

This is obviously right.
Comment 5 Matthias Clasen 2017-02-27 13:49:46 UTC
Review of attachment 346649 [details] [review]:

It does not hurt, but there should be no confusion: you are not allowed to pass NULL to this function. Adding these sorts of checks everywhere tends to make people think it is ok. It is not
Comment 6 Jan Tojnar 2017-02-27 14:18:25 UTC
Is there any other way to specify that NULL is invalid as a parameter? Ideally, there would be an explicit Option type and no NULL pointers but since this is C, I am afraid these assertions are necessary, unless we want segfaults.
Comment 7 Philip Withnall 2017-02-27 14:42:55 UTC
(In reply to Jan Tojnar from comment #6)
> Is there any other way to specify that NULL is invalid as a parameter?

Assume that NULL is not allowed as a parameter, unless a function argument is annotated with `(nullable)` in its documentation.

See https://wiki.gnome.org/Projects/GObjectIntrospection/Annotations.

If a function argument legitimately accepts NULL, and is missing a (nullable) annotation, that’s a bug. Note that common idioms like @cancellable and @error arguments are always assumed to be nullable, and don’t need explicit annotations. (There are a couple of other conventions too, but those are the main ones.)

> Ideally, there would be an explicit Option type and no NULL pointers but
> since this is C, I am afraid these assertions are necessary, unless we want
> segfaults.

These assertions turn a segfault into a critical warning. If the assertion is tripped, there is a bug in the caller — these assertions are not there to allow the caller to pass NULL in whenever it likes, they’re there to make it easier to debug when that does happen, so the bug in the caller can be fixed more easily.
Comment 8 Philip Withnall 2017-02-27 14:44:24 UTC
Attachment 346649 [details] pushed as 1a84511 - gio: Check for NUll when getting extensions