Re: [PATCH v2] string: add GNU basename(3)

[Corinna Vinschen <vinschen at redhat dot com>]

Hi Yaakov,

On Mar 16 11:41, Yaakov Selkowitz wrote:
> * libc/include/libgen.h (_BASENAME_DEFINED): Define.
> * libc/include/string.h (basename): Declare.
> * libc/string/Makefile.am (ELIX_4_SOURCES): Add gnu_basename.c.
> * libc/string/Makefile.in: Regenerate.
> * libc/string/gnu_basename.c: New file.

Thanks for the patch.  Just one point:

> +
> +char *
> +_DEFUN (__gnu_basename, (path),
> +	const char *path)
> +{
> +  char *p;
> +  if (path == NULL || *path == '\0')
> +    return "";
> +  p = (char *)path + strlen (path) - 1;
> +  if (*p == '/')
> +    return "";
> +  while (p >= path && *p != '/')
> +    p--;
> +  return p + 1;
> +}

Isn't that a bit, well, complicated?  Going backward from the end of the
string to the last slash is functionally equivalent to using strrchr
(path, '/').  That would simplify the code to:

  if ((p = strrchr (path, '/'))
    return p + 1;
  return path;

Am I missing something?


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

Attachment: pgpJwo51XjLib.pgp
Description: PGP signature