On Sat, May 30, 2009 at 10:05:19AM -0400, Jeff King wrote:
> 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
You know.. going with the compat/basename.c solution is probably
a better idea now that you've pointed out all these issues.
Then we can just use the system's basename() when available and
use our compat/basename.c otherwise (is it only for Windows?).
Here's basename() from libiberty.
It drops constness (likely to keep in line with the standard)
and has some #defines that I'll have to translate into git
equivalents (e.g. IS_DIR_SEPARATOR -> is_dir_sep).
Do we have a cross-platform ISALPHA thing (just thinking out
loud -- I can check). I'm under the impression that going with
something like this should get us in a better place.
What do you think?
char *basename (const char *name)
{
const char *base;
#if defined (HAVE_DOS_BASED_FILE_SYSTEM)
/* Skip over the disk name in MSDOS pathnames. */
if (ISALPHA (name[0]) && name[1] == ':')
name += 2;
#endif
for (base = name; *name; name++) {
if (IS_DIR_SEPARATOR (*name)) {
base = name + 1;
}
}
return (char *) base;
}
--
David
--
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