Eric Blake wrote: > On 09/04/2012 04:57 AM, Eric Blake wrote: >>> * lib/fts.c (fts_open): Upon initialization, if a name ends in two >>> or more slashes, trim all but the final one. But if a name consists >>> solely of two or more slashes, reduce it to "//", not to "/". >>> >> >> For two slashes, that is correct. But trimming "///" to "//" is a >> violation of POSIX and fails on cygwin; "///" must be the same as "/", >> not "///". >> >>> +++ b/lib/fts.c >>> @@ -487,6 +487,14 @@ fts_open (char * const *argv, >>> for (root = NULL, nitems = 0; *argv != NULL; ++argv, ++nitems) { >>> /* *Do* allow zero-length file names. */ >>> size_t len = strlen(*argv); >>> + >>> + /* If there are two or more trailing slashes, trim all but >>> one, >>> + but don't change "//" to "/". */ >>> + char const *v = *argv; >>> + if (2 < len && v[len - 1] == '/') >>> + while (2 < len && v[len - 2] == '/') >>> + --len; > > I think that this would do the right thing, though (notice that it > strips all but 1 slash for anything longer than "//" to begin with): > > + char const *v = *argv; > + if (2 < len && v[len - 1] == '/') > + while (1 < len && v[len - 1] == '/') > + --len;
Here's the revised commit: >From 3a9002d3cc63da7110f133b1040d2d2b0aad8305 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyer...@redhat.com> Date: Tue, 4 Sep 2012 11:36:38 +0200 Subject: [PATCH] fts: reduce two or more trailing slashes to just one, usually * lib/fts.c (fts_open): Upon initialization, if a name ends in two or more slashes, trim all but the final one. But if a name consists solely of two slashes, don't modify it. If it consists solely of three or more slashes, strip all but one. This is part of the solution to a minor problem with rm: it would print a bogus ELOOP diagnostic when failing to remove the slash-decorated name of a symlink-to-directory: $ mkdir d && ln -s d s && env rm -r s/ rm: cannot remove 's': Too many levels of symbolic links With the change below and a trivial don't-trim-trailing-slashes adjustment to remove.c, it does this: $ env rm -r s/ rm: cannot remove 's/': Not a directory Improved by: Eric Blake --- ChangeLog | 21 +++++++++++++++++++++ lib/fts.c | 8 ++++++++ 2 files changed, 29 insertions(+) diff --git a/ChangeLog b/ChangeLog index d4d2cb1..0874c81 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,26 @@ 2012-09-04 Jim Meyering <meyer...@redhat.com> + fts: reduce two or more trailing spaces to just one, usually + * lib/fts.c (fts_open): Upon initialization, if a name ends in two + or more slashes, trim all but the final one. But if a name consists + solely of two slashes, don't modify it. If it consists solely of + three or more slashes, strip all but one. + + This is part of the solution to a minor problem with rm: + it would print a bogus ELOOP diagnostic when failing to remove + the slash-decorated name of a symlink-to-directory: + + $ mkdir d && ln -s d s && env rm -r s/ + rm: cannot remove 's': Too many levels of symbolic links + + With the change below and a trivial don't-trim-trailing-slashes + adjustment to remove.c, it does this: + + $ env rm -r s/ + rm: cannot remove 's/': Not a directory + + Improved by: Eric Blake + fts: when there is no risk of overlap, use memcpy, not memmove * lib/fts.c (fts_alloc): Fix unjustified memcopy: s/memmove/memcpy/ diff --git a/lib/fts.c b/lib/fts.c index ce14a80..36f7e0d 100644 --- a/lib/fts.c +++ b/lib/fts.c @@ -487,6 +487,14 @@ fts_open (char * const *argv, for (root = NULL, nitems = 0; *argv != NULL; ++argv, ++nitems) { /* *Do* allow zero-length file names. */ size_t len = strlen(*argv); + + /* If there are two or more trailing slashes, trim all but one, + but don't change "//" to "/", and do map "///" to "/". */ + char const *v = *argv; + if (2 < len && v[len - 1] == '/') + while (1 < len && v[len - 2] == '/') + --len; + if ((p = fts_alloc(sp, *argv, len)) == NULL) goto mem3; p->fts_level = FTS_ROOTLEVEL; -- 1.7.12.176.g3fc0e4c