changeset 1471:b748127e092e

Bug report from luckboy: rm -f on a broken symlink didn't work because our "does it exist" test (to avoid errors on rm -f of nonexistent files) said it didn't. The fix: replace the access() with unlink(), which produces the same "does not exist" errno and has the added bonus of acting as a fastpath for rm -f on non-directories. (And since it produces a different error on directories, falls through to the old behavior there.) Most of this commit is comment updates explaining being subtle. :)
author Rob Landley <rob@landley.net>
date Sat, 13 Sep 2014 14:48:37 -0500
parents 4f25d3a3eda9
children 2f9bc9495144
files toys/posix/rm.c
diffstat 1 files changed, 7 insertions(+), 3 deletions(-) [+]
line wrap: on
line diff
--- a/toys/posix/rm.c	Thu Sep 11 20:50:10 2014 -0500
+++ b/toys/posix/rm.c	Sat Sep 13 14:48:37 2014 -0500
@@ -88,11 +88,15 @@
       continue;
     }
 
-    // There's a race here where a file removed between this access and
+    // Files that already don't exist aren't errors for -f, so try a quick
+    // unlink now to see if it succeeds or reports that it didn't exist.
+    if ((toys.optflags & FLAG_f) && (!unlink(*s) || errno == ENOENT))
+      continue;
+
+    // There's a race here where a file removed between the above check and
     // dirtree's stat would report the nonexistence as an error, but that's
     // not a normal "it didn't exist" so I'm ok with it.
-    if ((toys.optflags & FLAG_f) && (access(*s, F_OK) && errno == ENOENT))
-      continue;
+
     dirtree_read(*s, do_rm);
   }
 }