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 755046 - gfileutils: Add precondition checks to g_file_test()
gfileutils: Add precondition checks to g_file_test()
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2015-09-15 09:02 UTC by Philip Withnall
Modified: 2017-04-08 05:16 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gfileutils: Add precondition checks to g_file_test() (955 bytes, patch)
2015-09-15 09:02 UTC, Philip Withnall
committed Details | Review

Description Philip Withnall 2015-09-15 09:02:14 UTC
Trivial patch attached.
Comment 1 Philip Withnall 2015-09-15 09:02:18 UTC
Created attachment 311334 [details] [review]
gfileutils: Add precondition checks to g_file_test()

Otherwise g_file_test(NULL, …) causes a call to access(NULL, …) on
Linux, which is disallowed and valgrind complains about it.
Comment 2 Matthias Clasen 2015-09-21 12:34:16 UTC
I'm not a huge fan of slapping these onto everything. A != NULL check really adds very little, and it is not free.
Comment 3 Philip Withnall 2015-09-21 13:37:24 UTC
(In reply to Matthias Clasen from comment #2)
> A != NULL check really adds very little

It adds a runtime warning message when the programmer does something bad, and a convenient way of breaking on the error with G_DEBUG=fatal-warnings.

> and it is not free.

Isn’t that what G_DISABLE_CHECKS is for?
Comment 4 Matthias Clasen 2015-09-21 13:41:15 UTC
(In reply to Philip Withnall from comment #3)

> Isn’t that what G_DISABLE_CHECKS is for?

G_DISABLE_CHECKS disables _all_ runtime checks. The useless ones and the useful ones. Thats not an excuse for adding more and more useless ones...
Comment 5 Philip Withnall 2015-09-21 14:38:02 UTC
(In reply to Matthias Clasen from comment #4)
> (In reply to Philip Withnall from comment #3)
> 
> > Isn’t that what G_DISABLE_CHECKS is for?
> 
> G_DISABLE_CHECKS disables _all_ runtime checks. The useless ones and the
> useful ones. Thats not an excuse for adding more and more useless ones...

Then what’s the point in G_DISABLE_CHECKS? Runtime checks should never fail; if one does, that’s a programming error. As I understand it, if someone cares about runtime performance, they should set G_DISABLE_CHECKS for their release builds, and keep it unset for development builds.
Comment 6 Allison Karlitskaya (desrt) 2017-03-23 12:45:42 UTC
Review of attachment 311334 [details] [review]:

Sure
Comment 7 Matthias Clasen 2017-04-08 05:16:20 UTC
Attachment 311334 [details] pushed as a5b58da - gfileutils: Add precondition checks to g_file_test()