Re: [PATCH v5 2/3] path: add a find_basename() portability function

[Jeff King <peff@peff.net>]

On Fri, May 29, 2009 at 07:18:09PM -0700, David Aguilar wrote:

> +/* return the basename of a path */
> +const char *find_basename(const char *path)
> +{
> +	const char *basename = path + strlen(path) - 1;
> +	while(*basename && basename > path) {
> +		basename--;
> +		if (is_dir_sep(*basename)) {
> +			basename++;
> +			break;
> +		}
> +	}
> +	return basename;
> +}

Hmm. Is there any point to the *basename condition in the loop? By using
strlen, you are not going to go past any NULs, and you are already
checking that you don't go past the beginning of the string.

Speaking of which, how does this handle an input of ""? It seems that it
would return a pointer to one character before the string. Given your
loop, you need to special-case when *path is NUL.

Also, how should trailing dir_seps be handled? basename() will actually
return "" if given "foo/". Your implementation, when given "/foo/bar/"
will return "bar/" (and it must keep the trailing bit since we are
neither reallocating nor munging the input string). But given
"/foo/bar//", it will return simply "/". I could see an argument for
either "bar//" or "", but I think behaving differently for trailing "/"
versus trailing "//" doesn't make sense.

-Peff