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 678549 - Add some NULL-safe compare APIs
Add some NULL-safe compare APIs
Status: RESOLVED OBSOLETE
Product: glib
Classification: Platform
Component: general
unspecified
Other All
: Normal enhancement
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2012-06-21 15:12 UTC by Allison Karlitskaya (desrt)
Modified: 2018-05-24 14:17 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Add (and test) g_str_{hash,equal}0. (6.66 KB, patch)
2012-06-21 15:14 UTC, Allison Karlitskaya (desrt)
none Details | Review
Add (and test) g_variant_equal0(). (4.61 KB, patch)
2012-06-21 15:14 UTC, Allison Karlitskaya (desrt)
none Details | Review

Description Allison Karlitskaya (desrt) 2012-06-21 15:12:52 UTC
g_str_equal0, g_variant_equal0
Comment 1 Allison Karlitskaya (desrt) 2012-06-21 15:14:01 UTC
Created attachment 216935 [details] [review]
Add (and test) g_str_{hash,equal}0.

Also, modify the documentation for g_str_equal() to remove the
suggestion that g_strcmp0() is a better choice.  g_str_equal() is vastly
better in terms of readability and avoids the common logic problems
associated with use of strcmp-class functions.
Comment 2 Allison Karlitskaya (desrt) 2012-06-21 15:14:03 UTC
Created attachment 216936 [details] [review]
Add (and test) g_variant_equal0().
Comment 3 Colin Walters 2012-06-21 20:48:21 UTC
This could use a bit of motivation; is there any existing code in GLib or GNOME that wants to use this?
Comment 4 Emmanuele Bassi (:ebassi) 2012-06-21 23:07:29 UTC
lots of people use (g_strcmp0() == 0) in their conditions, so g_str_equal0() would at least improve readability; plus, I don't think changing the preconditions on g_str_equal() to accept NULL is a good idea either, so g_str_equal0() would also catch the (str != NULL && g_str_equal (str, "foo")) pattern.
Comment 5 Colin Walters 2012-06-22 00:02:33 UTC
(In reply to comment #4)
> lots of people use (g_strcmp0() == 0) in their conditions,

$ pwd
/src/glib
$ git grep 'g_strcmp0.*== 0' | wc -l
210
$ 

Looks right.

> so g_str_equal0()
> would at least improve readability; plus, I don't think changing the
> preconditions on g_str_equal() to accept NULL is a good idea either,

Agree with that.

 so
> g_str_equal0() would also catch the (str != NULL && g_str_equal (str, "foo"))
> pattern.

$ git grep '!= NULL &&.* g_str_equal' | wc -l
0
$

Anyways, my question was just about ensuring that we're trying to, for new API proposals, ensuring everyone involved in the discussion has a good idea of where and how exactly the new API would be used.
Comment 6 Allison Karlitskaya (desrt) 2012-06-22 03:19:12 UTC
On the GVariant side: David requested this one, and I've also found myself wanting it a lot.
Comment 7 Dan Winship 2012-06-22 14:32:29 UTC
(In reply to comment #4)
> I don't think changing the
> preconditions on g_str_equal() to accept NULL is a good idea either

Why? We had no choice but to make a new function for g_strcmp0(), but g_str_equal() will currently crash if either argument is NULL, so extending it to support NULL couldn't break any currently-working code.
Comment 8 Allison Karlitskaya (desrt) 2012-06-22 15:01:21 UTC
I prefer that g_str_equal() crashes.  The fact that it does has allowed me to very quickly find bugs.  In fact, the thing that led me to working on this patch in the first place was that I found NULL creeping in somewhere unexpected and had to change some other (related) code to check for it.
Comment 9 Colin Walters 2012-06-22 15:13:32 UTC
(In reply to comment #6)
> On the GVariant side: David requested this one, and I've also found myself
> wanting it a lot.

URLs/git repository names? 

(I know I'm being painful about this...sorry)
Comment 10 Christian Hergert 2015-05-18 05:50:17 UTC
We use this pattern heavily in Builder.

In fact, we have two that are quite useful:

 ide_str_equal0
 ide_str_empty0

I'd love to see them in glib directly. Any chance we can push this through?
Comment 11 Matthias Clasen 2015-05-18 12:17:48 UTC
My position on this should be well known: NULL is not a string

I much prefer to have an explicit if (str == NULL) case in the code before any use of string APIs.

Mixing NULL and "" in the api is only going to increase the confusion of programmers who are already confused about it.
Comment 12 Christian Hergert 2015-05-18 19:45:49 UTC
(In reply to Matthias Clasen from comment #11)
> My position on this should be well known: NULL is not a string
> 
> I much prefer to have an explicit if (str == NULL) case in the code before
> any use of string APIs.
> 
> Mixing NULL and "" in the api is only going to increase the confusion of
> programmers who are already confused about it.

For prior art, C# has String.IsNullOrEmpty() which I think still makes it clear that they are separate things.

I'm not suggesting that "g_str_equal0()" would equate NULL and "". What I find it convenient for is string setters for the same reasons as g_set_object(). It helps fight excessive notify emissions.

  if (!ide_str_equal0 (priv->a, a))
    {
      g_free (priv->a);
      priv->a = g_strdup (a);
      g_object_notify (self, "a");
    }

Now, if we had a more abstract way to write string properties, I do imagine most of my usage of this would disappear (but not all).
Comment 13 GNOME Infrastructure Team 2018-05-24 14:17:27 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glib/issues/558.