Re: Rationale for change in behavior of g_strsplit when passed empty string?

[Owen Taylor <otaylor redhat com>]

Joel Becker <jlbec evilplan org> writes:

> On Wed, Jul 18, 2001 at 04:33:17PM -0400, Owen Taylor wrote:
> > 
> > Joel Becker <jlbec evilplan org> writes:
> > 
> > > if ((strv == NULL) || (strv[0] == NULL))
> > > 
> > > 	as a method to check if the split was on an empty string.
> > 
> > Wait ... unless you have no idea whether you are using GLib-1.2 or
> > GLib-1.3, the || makes no sense. In either case it is deterministic.
> 
> 	How do you mean?  Under glib 1.2, strsplit can return NULL or 
> {NULL}.  Granted, I should know that my passed string is non-null, but
> it is a sanity check.  The day I assume that strsplit returned non-NULL,
> it'd return NULL and segfault :-)

The day you assume that g_strsplit() will "safely" handle NULL
with only a warning is the day that your user compiled GLib
with --disable-debug. And g_strsplit() segfaults  immediately.

g_return_if_fail() statements are a courtesy to the programmer
to provide a clear error message in a clear place. That doesn't
mean that the programmer is allowed to trigger them.

If you are checking to see if the return of g_strsplit() is
NULL, then your are cluttering your code with useless checks.

For any defined input, g_strsplit() never returns NULL. You
are, in no circumstances, allowed to call g_strsplit() with
a NULL string.

> 	Under the glib 1.3 behavior, this *is* a useless check.  It
> should instead check strv[0][0] and strv[1] as mentioned below.
>  
> > OK, I'm lost. If I wanted to check if the string I was splitting was ":",
> > I think I'd use strcmp (string, ":") == 0   ;-)
> 
> 	For that particular example, I have something along the line of
> ':' delimited input.  It may be "foo:bar", it may be ":bar", whatever.
> ":" being an emtpy case, but valid, whereas "" is invalid input.
> 	I guess your recommendation is that my code check for ":" and ""
> before calling strsplit.  I guess that would work, just have another
> level of indentation.

And what if your allowed input allowed for exactly _three_ fields?

Just write:

 gchar **fields = g_strsplit (input, ":");

 gint field_count = 0;
 while (fields[field_count])
   field_count++;

 if (field_count != 2)
   {
     g_warning ("Invalid input - must have exactly two fields\n");
   } 

Or something along those lines.  

> 	Anyway, I was just mentioning that the change modifies behavior
> in a way that will break some folks, and I could not see a benefit.  I
> suspect I'll just wait till it bites me :-)

Well, if you look at the bigger picture, what the current patch
does is:

 - Restores the old behavior "" => []
 - Keeps the change ":" => [ "", "" ]

Which you seem to expect from the above. :-)

It's actually a pretty darn big mess since the intended behavior
wasn't documented well and what was implemented was almost certainly
buggy.  We just have to make some best guess idea of what to do.

Regards,
                                        Owen