[PATCH] dirtree: fix 0b2cfcb8fdea9673f3c2e0940f1b16d5825e16ea.

0b2cfcb8fdea9673f3c2e0940f1b16d5825e16ea broke `ls -l` on files that
don't exist.

We didn't spot this because the corresponding ls test (which did exist)
had accidentally been mangled in a couple of ways.

This patch fixes the test (and extends it to differentiate between a
couple of possible ways it can go wrong), and fixes the broken path in
dirtree_add_node.

Bug: http://b/144698492
---
 lib/dirtree.c | 14 +++++++-------
 tests/ls.test |  5 +++--
 2 files changed, 10 insertions(+), 9 deletions(-)

On Wed, Nov 20, 2019 at 2:04 PM enh <[email protected]> wrote:
>
> this change broke `ls -l` for non-existent arguments:
>
> /tmp/toybox-dirtree$ ./toybox ls -l /asd
> b--------T 140728898420758 1964107300 476192747 2935, 304972
> 1969-12-31 16:00 /asd
> /tmp/toybox-dirtree$ git revert 0b2cfcb8fdea9673f3c2e0940f1b16d5825e16ea
> [master f83f87fa] Revert "Let "find -L -type -l" find dangling symlinks."
>  1 file changed, 6 insertions(+), 12 deletions(-)
> /tmp/toybox-dirtree$ make
> scripts/make.sh
> Generate headers from toys/*/*.c...
> Library probe...........
> Make generated/config.h from .config.
> Compile toybox......
> /tmp/toybox-dirtree$ ./toybox ls -l /asd
> ls: /asd: No such file or directory
> /tmp/toybox-dirtree$ ./toybox ls -l /asd
> ls: /asd: No such file or directory
> /tmp/toybox-dirtree$ ./toybox ls -l /asd
> ls: /asd: No such file or directory
>
>
>
> commit 0b2cfcb8fdea9673f3c2e0940f1b16d5825e16ea
> Author: Rob Landley <[email protected]>
> Date:   Tue Oct 29 10:59:46 2019 -0500
>
>     Let "find -L -type -l" find dangling symlinks.
>
> diff --git a/lib/dirtree.c b/lib/dirtree.c
> index df38b257..beaafd59 100644
> --- a/lib/dirtree.c
> +++ b/lib/dirtree.c
> @@ -32,12 +32,18 @@ struct dirtree *dirtree_add_node(struct dirtree *parent, 
> cha
> r *name, int flags)
>    int len = 0, linklen = 0, statless = 0;
>
>    if (name) {
> -    // open code this because haven't got node to call dirtree_parentfd() on 
> ye
> t
> -    int fd = parent ? parent->dirfd : AT_FDCWD;
> -
> -    if (fstatat(fd, name, 
> &st,AT_SYMLINK_NOFOLLOW*!(flags&DIRTREE_SYMFOLLOW)))
> {
> -      if (flags&DIRTREE_STATLESS) statless++;
> -      else goto error;
> +    // open code fd = because haven't got node to call dirtree_parentfd() on 
> ye
> t
> +    int fd = parent ? parent->dirfd : AT_FDCWD,
> +      sym = AT_SYMLINK_NOFOLLOW*!(flags&DIRTREE_SYMFOLLOW);
> +
> +    // stat dangling symlinks
> +    if (fstatat(fd, name, &st, sym)) {
> +      if (errno != ENOENT
> +        || (!sym && fstatat(fd, name, &st, AT_SYMLINK_NOFOLLOW)))
> +      {
> +        if (flags&DIRTREE_STATLESS) statless++;
> +        else goto error;
> +      }
>      }
>      if (S_ISLNK(st.st_mode)) {
>        if (0>(linklen = readlinkat(fd, name, libbuf, 4095))) goto error;
From b05ab1976f4a8f74cb2d13a5c9df7e8f6aac52a4 Mon Sep 17 00:00:00 2001
From: Elliott Hughes <[email protected]>
Date: Wed, 20 Nov 2019 14:25:34 -0800
Subject: [PATCH] dirtree: fix 0b2cfcb8fdea9673f3c2e0940f1b16d5825e16ea.

0b2cfcb8fdea9673f3c2e0940f1b16d5825e16ea broke `ls -l` on files that
don't exist.

We didn't spot this because the corresponding ls test (which did exist)
had accidentally been mangled in a couple of ways.

This patch fixes the test (and extends it to differentiate between a
couple of possible ways it can go wrong), and fixes the broken path in
dirtree_add_node.

Bug: http://b/144698492
---
 lib/dirtree.c | 14 +++++++-------
 tests/ls.test |  5 +++--
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/lib/dirtree.c b/lib/dirtree.c
index beaafd59..96c0de14 100644
--- a/lib/dirtree.c
+++ b/lib/dirtree.c
@@ -34,16 +34,16 @@ struct dirtree *dirtree_add_node(struct dirtree *parent, char *name, int flags)
   if (name) {
     // open code fd = because haven't got node to call dirtree_parentfd() on yet
     int fd = parent ? parent->dirfd : AT_FDCWD,
-      sym = AT_SYMLINK_NOFOLLOW*!(flags&DIRTREE_SYMFOLLOW);
+      sym = AT_SYMLINK_NOFOLLOW*!(flags&DIRTREE_SYMFOLLOW), okay = 1;
 
     // stat dangling symlinks
     if (fstatat(fd, name, &st, sym)) {
-      if (errno != ENOENT
-        || (!sym && fstatat(fd, name, &st, AT_SYMLINK_NOFOLLOW)))
-      {
-        if (flags&DIRTREE_STATLESS) statless++;
-        else goto error;
-      }
+      okay = (errno == ENOENT && !sym &&
+              !fstatat(fd, name, &st, AT_SYMLINK_NOFOLLOW));
+    }
+    if (!okay) {
+      if (flags&DIRTREE_STATLESS) statless++;
+      else goto error;
     }
     if (S_ISLNK(st.st_mode)) {
       if (0>(linklen = readlinkat(fd, name, libbuf, 4095))) goto error;
diff --git a/tests/ls.test b/tests/ls.test
index c207c3f6..ae6356a7 100755
--- a/tests/ls.test
+++ b/tests/ls.test
@@ -54,8 +54,9 @@ rm -rf lstest/* && touch lstest/file1.txt && INODE=`stat -c %i lstest/file1.txt`
 testing "with -i" "$IN && ls -i 2>/dev/null; $OUT" "$INODE file1.txt\n" "" ""
 unset INODE
 
-testing "missing" "$IN && ls does-not-exist 2>err ; grep -q 'ls:.*missing.*: No
-such file' err || echo missing error; $OUT" "" "" ""
+testing "missing" "$IN && ls non-existent 2>err && echo oops1 ; \
+  grep -q 'ls:.*non-existent.*: No such file' err || echo oops2; $OUT" \
+  "" "" ""
 
 rm -f lstest/{file1.txt,err}
 touch lstest/{one,two,three,four,five,six,seven,eight,nine,ten}
-- 
2.24.0.432.g9d3f5f5b63-goog

_______________________________________________
Toybox mailing list
[email protected]
http://lists.landley.net/listinfo.cgi/toybox-landley.net

Reply via email to