Re: g_strjoinv() is very slow

[Raja R Harinath <harinath cs umn edu>]

David Odin <David Odin bigfoot com> writes:
> On Sun, Sep 24, 2000 at 04:58:15PM +0200, David Odin wrote:
> > On Sat, Sep 23, 2000 at 10:45:41PM -0400, Havoc Pennington wrote:
> > > David Odin <David Odin bigfoot com> writes: 
> > > >   Actually, it is a very good idea, but I think isn't very portable on
> > > > non-GNU platforms. 
> > > > 
> > > >   I didn't know this function, and I saw this from the man page:
> > > > CONFORMING TO
> > > >        This function is not part of the ANSI or POSIX  standards,
> > > >        and  is  not  customary  on Unix systems, but is not a GNU
> > > >        invention either.  Perhaps it comes from MS-DOS.
> > > >     
> > > >   Havoc, what do you think? Should I re-write my patch to use
> > > > stpcpy() and adding a test to configure.in?
> > > 
> > > If you did, you'd just write a g_stpcpy(), which would be implemented
> > > depending on a configure.in test, then use that.
> > > 
> > > I don't have any opinion though whether it's better to use
> > > it. Probably only if we want g_stpcpy() in GLib anyway, and I don't
> > > know whether we do or not.

The decision to use stpcpy is IMO independent of providing g_stpcpy().
It is quite a simple function, and is usually a better alternative to
strcat (at least Ulrich Drepper says so :-) -- so we should use it.

> diff -Nrudb glib.orig/glib.h glib/glib.h
> --- glib.orig/glib.h	Tue Sep 19 16:30:35 2000
> +++ glib/glib.h	Sun Sep 24 16:21:51 2000
> @@ -1761,8 +1761,8 @@
>  gchar*   g_strjoinv		(const gchar  *separator,
>  				 gchar       **str_array);
>  void     g_strfreev		(gchar       **str_array);
> -
> -
> +gchar*   stpcpy       (gchar *dest,
> +                       const char *src);

That should be g_stpcpy.

>        separator_len = strlen (separator);
>        len = 1 + strlen (str_array[0]);
> -      for(i = 1; str_array[i] != NULL; i++)
> -	len += separator_len + strlen(str_array[i]);
> +      for (i = 1; str_array[i] != NULL; i++)
> +        len += strlen(str_array[i]);
> +      len += separator_len*i;

Should it be

       len += separator_len * (i - 1);

>        string = g_new (gchar, len);
>        *string = 0;
>        strcat (string, *str_array);
> +      ptr = string + strlen (*str_array);

Maybe just replace it with

      string = g_new (gchar, len);
      ptr = g_stpcpy (string, *str_array);

>        for (i = 1; str_array[i] != NULL; i++)
>  	{
> -	  strcat (string, separator);
> -	  strcat (string, str_array[i]);
> +          ptr = g_stpcpy (ptr, separator);
> +          ptr = g_stpcpy (ptr, str_array[i]);

GLibc uses a more compact:

      ptr = g_stpcpy (g_stpcpy (ptr, separator), str_array[i]);

>        s = va_arg (args, gchar*);
>        strcat (string, s);
> +      ptr = string + strlen (s);
>  
>        s = va_arg (args, gchar*);
>        while (s)
>  	{
> -	  strcat (string, separator);
> -	  strcat (string, s);
> +          ptr = g_stpcpy(ptr, separator);
> +          ptr = g_stpcpy(ptr, s);

Similar comments. 

- Hari
-- 
Raja R Harinath ------------------------------ harinath cs umn edu
"When all else fails, read the instructions."      -- Cahn's Axiom
"Our policy is, when in doubt, do the right thing."   -- Roy L Ash