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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]