[PATCH] reimplement dirname v2

Tito farmatito at tiscali.it
Sun Sep 11 07:55:21 UTC 2011


On Saturday 10 September 2011 03:11:06 you wrote:
> On Friday 09 September 2011 23:17, Tito wrote:
> > > Does standard dirname fails for you on any of these examples?
> > > IOW: are you fixing a bug?
> > 
> > No, standard dirname() passes the tests with the same results.
> > 
> > > >  * This example is added by me:
> > > >  * NULL           "."
> > > 
> > > > 4) you don't need to declare libgen.h
> > > 
> > > This is good because ... ?
> > 
> > We have already a private implementation of basename
> > in libbb/get_last_path_component.c.
> 
> The reason for bb_basename() is unlike dirname, there are _two_ different_
> versions of basename()! -
> 
> NOTES
>        There  are  two  different  versions  of basename() - the POSIX version
>        described above, and the GNU version one gets after
>          #define _GNU_SOURCE
>          #include <string.h>
>        The GNU version never modifies its  argument,  and  returns  the  empty
>        string  when  path has a trailing slash, and in particular also when it
>        is "/".  There is no GNU version of dirname().
> 
> Seeing the above, I feared we can't be 100% sure we are getting
> basename() we are expecting to get.
> 
> 
> > so we would need libgen.h only for dirname
> > (in other words one more header we don't need to care
> > about across different systems).
> 
> This would make sense if there are libcs with problematic libgen.h.
> Otherwise, just removing the need to include one header
> is not a win per se: we'll need to care about bugs in
> our implementation instead.
> 
> How many other functions you want to reimplement for bbox?
> strnlen? strcasecmp? Where to stop?
> 
> I think a better way to spend your time in such cases is to improve
> uclibc's code directly, making not only busybox, but many other
> projects able to reap benefits.
> 
> In this particular case, look into libc/string/dirname.c
> in uclibc tree.
> 
> I can commit your changes into uclibc tree.
> 
8-) you want me to attempt to break uclibc?!!? ;-)

Never thought about that.... 

Jokes aside, I've experimented a little
with a better dirname V3 for busybox:

char* FAST_FUNC xmalloc_dirname(const char *path) {
        char *s = xstrdup(path);
        char *p = dirname(s);
        if (p != s) {
                free(s);
                return xstrdup(p);
        }
        return p;
}

That always returns malloced path and doesn't leak memory
but there was not the size reduction I expected as there
are not enough suitable dirname occurrences in the bb codebase.
So for the time being I'll drop the subject unless there is some
new better idea about it.  
There are also two more problems of different behaviour
between linux and freebsd:
1) freebsd dirname could return NULL on error
2) It errors on path component to be returned larger than MAXPATHLEN
  meanwhile linux dirname swallows happily args larger than PATH_MAX
  (could not find MAXPATHLEN for linux so I assume it is PATH_MAX for us).
Even if this could happen only on corner cases it could eventually interact badly
with bb's code as we don't expect dirname to return NULL, at least the manpages
don't mention it.

Ciao,
Tito


More information about the busybox mailing list