Re: bugs in dirname module

[Eric Blake]

> Paul Eggert <address@hidden> wrote:
> > If we go this route, then base_name(F) cannot in general yield a
> > suffix of F even on Unix systems, since we would want dir_name("a/b/")
> > == "a/b" and base_name("a/b/") == ".".  Hence base_name will need to
> > allocate memory in general, even on Unix.  On Cygwin it will need it
> > to compute "./a:b".
> 
> That's a big departure from established tradition.
> Do you really want to call the result base_name?
> The first few uses of base_name in the coreutils that I looked at
> do not seem amenable to the proposed change in semantics.  E.g.,
> 
> /* Return true if the last component of NAME is `.' or `..'
>    This is so we don't try to recurse on `././././. ...' */
> static bool
> basename_is_dot_or_dotdot (const char *name)
> {
>   char const *base = base_name (name);
>   return DOT_OR_DOTDOT (base);
> }

I think this argues for two functions - one which malloc's, and
one which returns a pointer within the string of the last
component.  I would like to see base_name() malloc (so that
it is symmetrical with dir_name), and add a new function
last_component() that does what base_name() currently
provides.  Then usages like basename_is_dot_or_dotdot
would switch to use last_component(), whereas usages like
basename(1) would add in the appropriate free() after
using base_name().

Also, I already audited gnulib, coreutils, tar, and findutils
for all usages of base_name, so I am prepared to provide
this segregation into base_name() vs. last_component()
as part of my patch.

> 
> > Also, src/dirname.c and src/basename.c will have to be modified to
> > strip redundant trailing slashes before invoking dir_name and
> > base_name.
> 
> Examples like the above (from remove.c) would also require removing
> trailing slashes.  So we'd have to make a writable copy and remove
> trailing slashes in order to be able to use the new base_name function,
> which would return a malloc'd result.  And we'd have to free it, too,
> of course.

strip_trailing_slashes already exists, and already assumes that
its input is writable.  So
"char *foo = strip_trailing_slashes (base_name (bar));"
is a nice replacement for the current
"char *foo = strip_trailing_slashes (strdup (base_name (bar)));"

--
Eric Blake