Jim Meyering wrote: > Steven Drake wrote: >> Summary: rm -f gives an error when trying to delete a non-existent >> file on a read-only filesystem >> >> Eg: >> /bin/rm: cannot remove `non-existent-file': Read-only file system. >> >> On a linux/glibc system the unlinkat syscall will set errno to EROFS and >> nonexistent_file_errno() does not perform any checks >> for this case. (Another rm does an fts_open/fts_read and ignores >> the file if fts_info is FST_NS before trying to unlink it.) > > Thanks for reporting that. > It also affected the very latest versions. > > I expect to fix it with this patch once I've added a test.
Here's the change+test pair I'll push shortly: >From 7bf2e3db23bd0a9ed59d95a683edd188fa52a033 Mon Sep 17 00:00:00 2001 From: Jim Meyering <[email protected]> Date: Tue, 3 Nov 2009 12:01:40 +0100 Subject: [PATCH 1/2] rm -f: ignore EROFS when it's really ENOENT rm -f must not print a diagnostic for a nonexistent file. However, most linux-based kernel unlinkat functions set errno to EROFS when the named file (regardless of whether it exists) would lie on a read-only file system. remove.c now performs an extra fstatat call in that case, to determine whether the file exists. * src/remove.c (excise): Map EROFS to ENOENT, if a file is nonexistent. Reported by Steven Drake in <http://savannah.gnu.org/bugs/?27923>. * NEWS (Changes in behavior): Mention it. --- NEWS | 6 ++++++ THANKS | 1 + src/remove.c | 12 ++++++++++++ 3 files changed, 19 insertions(+), 0 deletions(-) diff --git a/NEWS b/NEWS index 0760775..03ed83f 100644 --- a/NEWS +++ b/NEWS @@ -49,6 +49,12 @@ GNU coreutils NEWS -*- outline -*- echo and printf now interpret \e as the Escape character (0x1B). + rm -f /read-only-fs/nonexistent now succeeds and prints no diagnostic + on systems with an unlinkat syscall that sets errno to EROFS in that case. + Before, it would fail with a "Read-only file system" diagnostic. + Also, "rm /read-only-fs/nonexistent" now reports "file not found" rather + than the less precise "Read-only file system" error. + ** New features env and printenv now accept the option --null (-0), as a means to diff --git a/THANKS b/THANKS index 5efe2fa..c583e2a 100644 --- a/THANKS +++ b/THANKS @@ -541,6 +541,7 @@ Stephen Smoogen [email protected] Steve McConnel [email protected] Steve McIntyre [email protected] Steve Ward [email protected] +Steven Drake [email protected] Steven G. Johnson [email protected] Steven Mocking [email protected] Steven Parkes [email protected] diff --git a/src/remove.c b/src/remove.c index 87fb32b..c4b93fe 100644 --- a/src/remove.c +++ b/src/remove.c @@ -437,6 +437,18 @@ excise (FTS *fts, FTSENT *ent, struct rm_options const *x, bool is_dir) return RM_OK; } + /* The unlinkat from kernels like linux-2.6.32 reports EROFS even for + nonexistent files. When the file is indeed missing, map that to ENOENT, + so that rm -f ignores it, as required. Even without -f, this is useful + because it makes rm print the more precise diagnostic. */ + if (errno == EROFS) + { + struct stat st; + if ( ! (lstatat (fts->fts_cwd_fd, ent->fts_accpath, &st) + && errno == ENOENT)) + errno = EROFS; + } + if (ignorable_missing (x, errno)) return RM_OK; -- 1.6.5.2.292.g1cda2 >From d4b96c26ce0e79cb64fc99dda56d795765d5656d Mon Sep 17 00:00:00 2001 From: Jim Meyering <[email protected]> Date: Tue, 3 Nov 2009 14:30:56 +0100 Subject: [PATCH 2/2] tests: rm: add test for today's change in behavior * tests/Makefile.am (root_tests): Add rm/read-only to the list. * tests/rm/read-only: New file. --- tests/Makefile.am | 1 + tests/rm/read-only | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 0 deletions(-) create mode 100755 tests/rm/read-only diff --git a/tests/Makefile.am b/tests/Makefile.am index eec31ae..f67b796 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -37,6 +37,7 @@ root_tests = \ rm/fail-2eperm \ rm/no-give-up \ rm/one-file-system \ + rm/read-only \ tail-2/append-only \ touch/now-owned-by-other diff --git a/tests/rm/read-only b/tests/rm/read-only new file mode 100755 index 0000000..3c83aad --- /dev/null +++ b/tests/rm/read-only @@ -0,0 +1,56 @@ +#!/bin/sh +# Ensure that rm -f nonexistent-file-on-read-only-fs succeeds. + +# Copyright (C) 2009 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 3 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, see <http://www.gnu.org/licenses/>. + +if test "$VERBOSE" = yes; then + set -x + rm --version +fi + +. $srcdir/test-lib.sh +require_root_ + +cwd=`pwd` +cleanup_() { cd /; umount "$cwd/mnt"; } + +skip=0 +# Create a file system, then mount it. +dd if=/dev/zero of=blob bs=8192 count=200 > /dev/null 2>&1 \ + || skip=1 +mkdir mnt || skip=1 +mkfs -t ext2 -F blob \ + || skip_test_ "failed to create ext2 file system" + +mount -oloop blob mnt || skip=1 +echo test > mnt/f || skip=1 +test -s mnt/f || skip=1 +mount -o remount,loop,ro mnt || skip=1 + +test $skip = 1 \ + && skip_test_ "insufficient mount/ext2 support" + +# Applying rm -f to a nonexistent file on a read-only file system must succeed. +rm -f mnt/no-such > out 2>&1 || fail=1 +# It must produce no diagnostic. +test -s out && fail=1 + +# However, trying to remove an existing file must fail. +rm -f mnt/f > out 2>&1 && fail=1 +# with a diagnostic. +test -s out || fail=1 + +Exit $fail -- 1.6.5.2.292.g1cda2
