From a4d470409f5590b95d33ab67e5866ab71e4f0e27 Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Fri, 29 Sep 2006 09:54:24 +0000 Subject: [PATCH] Work around a readdir bug in Darwin 7.9.0 (MacOS X 10.3.9) on HFS+ and NFS, whereby rm would not remove all files in a directory. * src/remove.c (CONSECUTIVE_READDIR_UNLINK_THRESHOLD): Reduce to 10. (NEED_REWIND): New macro, so that we incur the cost of the work-around rewinddir only on afflicted systems. * NEWS: Clarify and correct. * tests/rm/readdir-bug: New file. Test for the above fix. * tests/rm/Makefile.am (TESTS): Add it. Prompted by testing and analysis from Bruno Haible: http://lists.gnu.org/archive/html/bug-coreutils/2006-09/msg00326.html --- ChangeLog | 13 ++++++++++++ NEWS | 5 +++-- src/remove.c | 30 +++++++++++++++----------- tests/rm/Makefile.am | 1 + tests/rm/readdir-bug | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 95 insertions(+), 14 deletions(-) create mode 100755 tests/rm/readdir-bug diff --git a/ChangeLog b/ChangeLog index f9bb134..7a79e0b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,16 @@ +2006-09-29 Jim Meyering + + Work around a readdir bug in Darwin 7.9.0 (MacOS X 10.3.9) on HFS+ + and NFS, whereby rm would not remove all files in a directory. + * src/remove.c (CONSECUTIVE_READDIR_UNLINK_THRESHOLD): Reduce to 10. + (NEED_REWIND): New macro, so that we incur the cost of the work-around + rewinddir only on afflicted systems. + * NEWS: Clarify and correct. + * tests/rm/readdir-bug: New file. Test for the above fix. + * tests/rm/Makefile.am (TESTS): Add it. + Prompted by testing and analysis from Bruno Haible: + http://lists.gnu.org/archive/html/bug-coreutils/2006-09/msg00326.html + 2006-09-28 Paul Eggert * tests/rm/fail-eperm: Unset BASH_ENV, CDPATH, and ENV, too; diff --git a/NEWS b/NEWS index ce72808..7d2eb9f 100644 --- a/NEWS +++ b/NEWS @@ -4,8 +4,9 @@ GNU coreutils NEWS -*- outline -*- ** Improved robustness - rm works around a bug in Darwin 8.6.1 w/NFS that kept it from removing - a directory containing 188 or more entries. + rm works around a bug in Darwin 7.9.0 (MacOS X 10.3.9) that would + sometimes keep it from removing all entries in a directory on an HFS+ + or NFS-mounted partition. sort would fail to handle very large input (around 40GB) on systems with a mkstemp function that returns a file descriptor limited to 32-bit offsets. diff --git a/src/remove.c b/src/remove.c index 24426cf..1ea7773 100644 --- a/src/remove.c +++ b/src/remove.c @@ -50,20 +50,27 @@ #define obstack_chunk_free free /* This is the maximum number of consecutive readdir/unlink calls that - can be made (with no intervening rewinddir or closedir/opendir) - before triggering a bug that makes readdir return NULL even though - some directory entries have not been processed. The bug afflicts - SunOS's readdir when applied to ufs file systems and Darwin 6.5's - (and OSX v.10.3.8's) HFS+. This maximum is conservative in that - demonstrating the problem seems to require a directory containing - at least 254 deletable entries (which doesn't count . and ..). - However, in 2006, we see that Darwin 8.6.1, using NFS has an even - lower limit: 188. */ + can be made (with no intervening rewinddir or closedir/opendir) before + triggering a bug that makes readdir return NULL even though some + directory entries have not been processed. The bug afflicts SunOS's + readdir when applied to ufs file systems and Darwin 6.5's (and OSX + v.10.3.8's) HFS+. This maximum is conservative in that demonstrating + the problem requires a directory containing at least 16 deletable + entries (which doesn't count . and ..). + This problem also affects Darwin 7.9.0 (aka MacOS X 10.3.9) on HFS+ + and NFS-mounted file systems, but not vfat ones. */ enum { - CONSECUTIVE_READDIR_UNLINK_THRESHOLD = 180 + CONSECUTIVE_READDIR_UNLINK_THRESHOLD = 10 }; +#ifdef HAVE_WORKING_READDIR +# define NEED_REWIND(readdir_unlink_count) 0 +#else +# define NEED_REWIND(readdir_unlink_count) \ + (CONSECUTIVE_READDIR_UNLINK_THRESHOLD <= (readdir_unlink_count)) +#endif + enum Ternary { T_UNKNOWN = 2, @@ -1139,8 +1146,7 @@ remove_cwd_entries (DIR **dirp, { /* fall through */ } - else if (CONSECUTIVE_READDIR_UNLINK_THRESHOLD - < n_unlinked_since_opendir_or_last_rewind) + else if (NEED_REWIND (n_unlinked_since_opendir_or_last_rewind)) { /* Call rewinddir if we've called unlink or rmdir so many times (since the opendir or the previous rewinddir) that this diff --git a/tests/rm/Makefile.am b/tests/rm/Makefile.am index 958b03e..8fc7bca 100644 --- a/tests/rm/Makefile.am +++ b/tests/rm/Makefile.am @@ -21,6 +21,7 @@ AUTOMAKE_OPTIONS = 1.1 gnits TESTS = \ + readdir-bug \ empty-inacc \ dir-nonrecur \ dot-rel \ diff --git a/tests/rm/readdir-bug b/tests/rm/readdir-bug new file mode 100755 index 0000000..b15ad20 --- /dev/null +++ b/tests/rm/readdir-bug @@ -0,0 +1,60 @@ +#!/bin/sh +# Exercise the Darwin/MacOS bug worked around on 2006-09-29, +# whereby rm would fail to remove all entries in a directory. + +# Copyright (C) 2006 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. + +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. + +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA +# 02110-1301, USA. + +if test "$VERBOSE" = yes; then + set -x + rm --version +fi + +pwd=`pwd` +t0=`echo "$0"|sed 's,.*/,,'`.tmp; tmp=$t0/$$ +trap 'status=$?; cd $pwd; chmod -R u+rwx $t0; rm -rf $t0 && exit $status' 0 +trap '(exit $?); exit $?' 1 2 13 15 + +framework_failure=0 +mkdir -p $tmp || framework_failure=1 +cd $tmp || framework_failure=1 + +# Create a directory containing many files. +# What counts is a combination of the number of files and +# the lengths of their names. For details, see +# http://lists.gnu.org/archive/html/bug-coreutils/2006-09/msg00326.html +mkdir b || framework_failure=1 +cd b || framework_failure=1 +for i in $(seq 1 250); do + touch $(printf %040d $i) || framework_failure=1 +done +cd .. || framework_failure=1 + +if test $framework_failure = 1; then + echo "$0: failure in testing framework" 1>&2 + (exit 1); exit 1 +fi + +fail=0 + +# On a buggy system, this would fail with the diagnostic, +# "cannot remove directory `b': Directory not empty" +rm -rf b || fail=1 + +test -d b && fail=1 + +(exit $fail); exit $fail -- 2.7.4