GNOME Bugzilla – Bug 606415
Use g_strcmp0 instead of safe_strcmp
Last modified: 2010-03-17 02:02:11 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.
Created attachment 151041 [details] [review] Use g_strcmp0 instead of safe_strcmp
Comment on attachment 151041 [details] [review] Use g_strcmp0 instead of safe_strcmp looks fine.
Comment on attachment 151041 [details] [review] Use g_strcmp0 instead of safe_strcmp commit f5b21802bbf02d89c61633286492f62109b0f2a2
This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you Rafal!
That was really fast, I'm really impressed! But what about using g_strcmp0 in gtkprintsettings.c? See the patch below:
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.
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 on attachment 151074 [details] [review] Use g_strcmp0 where it makes sense Sorry for the delay Rafal, just committed. commit 839c88fd7382e260b5d987423785baa5657356fe
I think we can close this bug now. Thank you Rafal.