Re: Glib proposal: add strlcpy and strlcat to glib

[Tim Janik <timj gtk org>]

On Fri, 5 May 2000, David Wheeler wrote:

> 
> Okay, here's my cut at porting strlcpy/strlcat to glib.
> 
> In glib they'll be called g_strlcpy/g_strlcat, for glib naming consistency.
> If strlcpy/strlcat already exists on your machine in <string.h>
> (true for Sun Solaris, as well as for several BSDs such as OpenBSD),
> then they're called. Otherwise, an implementation is compiled
> into the glib library and called when requested.
> 
> This patch modifies:
> * gstrfuncs.c (with the implementation),
> * glib.h (with the prototypes),
> * testglib.c (adding torture tests for them),
> * ChangeLog (describing the change),
> * configure.in and acconfig.h (for detecting if strlcpy/strlcat exist).
> 
> It _seems_ to work for me, but I'm only human.  Comments welcome!
> If there are no complaints, I'll throw it into the "incoming"
> area (or tell me if you'd prefer me do something different).

posting patches here for review is fine for now ;)
owen seems to prefer incoming/ but i am more likely to review
patches on gtk-devel-list.

your patch seems technically ok, so i've got mostly stylistic comments.


> --- glib.orig/gstrfuncs.c	Wed Mar  1 04:44:09 2000
> +++ glib/gstrfuncs.c	Fri May  5 00:54:24 2000
> @@ -1323,6 +1323,115 @@
>    return string;
>  }
>  
> +/*
> + * Functions g_strlcpy and g_strlcat were originally developed by 
> + * Todd C. Miller <Todd.Miller@courtesan.com>.
> + * See ftp://ftp.openbsd.org/pub/OpenBSD/src/lib/libc/string/strlcpy.3
> + * for more information.
> + */
> +
> +/* Use the native ones, if available; they might be implemented in assembly */
> +
> +#ifdef HAVE_STRLCPY
> +
> +size_t
> +g_strlcpy(char *dst, const char *src, size_t siz)
> +{
> +  return strlcpy(dst, src, siz);
> +}

use gsize and gchar for glib functions.
also, we align arguments by type and name vertically, and
we roughly follow gnu coding style which puts blanks before
function/macro paranthesis. (and you may be more elaborate
with variable names ;) all in all this becomes:

gsize
g_strlcpy (gchar       *dest,
           const gchar *src,
           gsize        dest_length)
{
  return strlcpy (dest, src, dest_length);
}


> +#else /* ! HAVE_STRLCPY */


> +size_t
> +g_strlcpy(char *dst, const char *src, size_t siz)
> +{
> +  register char *d = dst;
> +  register const char *s = src;
> +  register size_t n = siz;

you need to
 g_return_val_if_fail (dst != NULL, 0);
 g_return_val_if_fail (src != NULL, 0);
if you want to later access them. (eventhough the original strlcpy()
will probably segfault upon that, glib should never do that, instead
we try to be verbose if something unexpectedly happens).

> +
> +  /* Copy as many bytes as will fit */
> +  if (n != 0 && --n != 0) {

please put newlines before '{'

> +    do {
> +      if ((*d++ = *s++) == '\0')

we don't use inline assignments, so this needs to be split
up into seperate lines.

> +        break;
> +    } while (--n != 0);

and put newlines after '}'

> +  }
> +
> +  /* Not enough room in dst, add NUL and traverse rest of src */
> +  if (n == 0) {
> +    if (siz != 0)
> +      *d = '\0';    /* NUL-terminate dst */
> +    while (*s++)
> +      ;
> +  }
> +
> +  return(s - src - 1);  /* count does not include NUL */

blank before parantheses, but since return isn't actually a function,
you don't need them anyways and can just
  return s - src - 1;  /* count does not include NUL */

> +}

> +#endif /* HAVE_STRLCPY */

this is actually the end of the branch getting compiled when we
don't have strlcpy(), so make this:
#endif /* ! HAVE_STRLCPY */

>  gchar**
>  g_strsplit (const gchar *string,
>  	    const gchar *delimiter,
> --- glib.orig/glib.h	Fri Apr 28 08:24:52 2000
> +++ glib/glib.h	Tue May  2 00:03:44 2000
> @@ -78,6 +78,9 @@
>   */
>  #include <stdarg.h>
>  
> +/* include stddef for the definition of size_t */
> +#include <stddef.h>
> +

you don#t need this when you use gsize.

>  /* optionally feature DMALLOC memory allocation debugger
>   */
>  #ifdef USE_DMALLOC
> @@ -1585,6 +1588,10 @@
>  gchar*  g_strchomp              (gchar        *string);
>  /* removes leading & trailing spaces */
>  #define g_strstrip( string )	g_strchomp (g_strchug (string))
> +
> +size_t	 g_strlcpy		(char *dst, const char *src, size_t size);
> +size_t	 g_strlcat		(char *dst, const char *src, size_t size);
> +

this should be (similar to the above):
gsize g_strlcpy (gchar       *dest,
                 const gchar *src,
                 gsize        dest_length);


---
ciaoTJ