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 606415 - Use g_strcmp0 instead of safe_strcmp
Use g_strcmp0 instead of safe_strcmp
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Printing
2.19.x
Other All
: Normal trivial
: ---
Assigned To: gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2010-01-08 14:45 UTC by Rafal Luzynski
Modified: 2010-03-17 02:02 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Use g_strcmp0 instead of safe_strcmp (2.56 KB, patch)
2010-01-08 16:47 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
Use g_strcmp0 where it makes sense (1.10 KB, patch)
2010-01-09 00:30 UTC, Rafal Luzynski
committed Details | Review

Description Rafal Luzynski 2010-01-08 14:45:07 UTC
Why should we need a custom static function safe_strcmp if we have the GLib's g_strcmp0 function which seems to provide the same functionality with likely more safety and consistency? safe_strcmp is defined as the static function in two files: gtkprinter.c and gtkprinteroptionset.c. Also gtkprintsettings.c uses the idiom:

if (str != NULL && strcmp (str, "string") == 0)

which can be replaced with a simple call of g_strcmp0.

This bugfix will not change much but will remove some duplicated code.
Comment 1 Javier Jardón (IRC: jjardon) 2010-01-08 16:47:01 UTC
Created attachment 151041 [details] [review]
Use g_strcmp0 instead of safe_strcmp
Comment 2 Matthias Clasen 2010-01-08 16:58:34 UTC
Comment on attachment 151041 [details] [review]
Use g_strcmp0 instead of safe_strcmp

looks fine.
Comment 3 Javier Jardón (IRC: jjardon) 2010-01-08 17:28:21 UTC
Comment on attachment 151041 [details] [review]
Use g_strcmp0 instead of safe_strcmp

commit f5b21802bbf02d89c61633286492f62109b0f2a2
Comment 4 Javier Jardón (IRC: jjardon) 2010-01-08 17:28:54 UTC
This problem has been fixed in the development version. The fix will be available in the next major software release.

Thank you Rafal!
Comment 5 Rafal Luzynski 2010-01-09 00:23:45 UTC
That was really fast, I'm really impressed! But what about using g_strcmp0 in gtkprintsettings.c? See the patch below:
Comment 6 Rafal Luzynski 2010-01-09 00:30:06 UTC
Created attachment 151074 [details] [review]
Use g_strcmp0 where it makes sense

Use g_strcmp0 instead of if (val != NULL && strcmp...) in some places. However, there are still some places left unchanged where NULL value can be easily detected immediately instead of checking multiple times with g_strcmp0 before determining that NULL is not equal to any of the strings compared with. In such cases I don't find g_strcmp0 more optimal.
Comment 7 Rafal Luzynski 2010-03-16 14:55:30 UTC
Any comments about my second patch? Javier, your patch fixes the problem where it was obviously necessary. My fixes are not so necessary so it is OK if you accept it and also OK if you reject it and close the bug. But it's not so fine if you just ignore this because the bug is left open and makes GTK look more buggy than it actually is. :-)
Comment 8 Javier Jardón (IRC: jjardon) 2010-03-16 18:03:17 UTC
Comment on attachment 151074 [details] [review]
Use g_strcmp0 where it makes sense

Sorry for the delay Rafal, just committed.

commit 839c88fd7382e260b5d987423785baa5657356fe
Comment 9 Javier Jardón (IRC: jjardon) 2010-03-17 02:02:11 UTC
I think we can close this bug now. Thank you Rafal.