1
0
Fork 0
mirror of git://git.code.sf.net/p/cdesktopenv/code synced 2025-03-09 15:50:02 +00:00

Backport the ksh93v- rm builtin to fix 'rm -d' (#348)

The -d flag implemented in the rm builtin is completely broken. No
matter what you do it refuses to remove directories, even if -r is
also passed. Reproducer:

  $ mkdir /tmp/empty
  $ PATH=/opt/ast/bin rm -d /tmp/empty
  rm: /tmp/empty: directory
  $ PATH=/opt/ast/bin rm -dr /tmp/empty
  rm: /tmp/empty: directory not removed [Is a directory]

Additionally, the description of 'rm -d' in the man page contradicts
how it's specified in <https://www.austingroupbugs.net/view.php?id=802>.

The ksh93v- rm builtin fixed nearly all of these issues, so I've
backported it to 93u+m and applied one additional fix for 'rm -rd'.

src/lib/libcmd/rm.c:
- Backported the fixes from the ksh93v- rm builtin's -d flag when
  used on empty directories.
- Backported the man page update for rm(1) from ksh93v-.
- The ksh93v- rm builtin had one additional bug that caused the -r
  option to fail when combined with -d. This was fixed by
  overriding -d if -r is also passed.

src/cmd/ksh93/tests/builtins.sh:
- Add regression tests for the rm builtin's -d option.
This commit is contained in:
Johnothan King 2021-11-24 18:45:21 -08:00 committed by Martijn Dekker
parent 2d65148fad
commit 84ded2d0c4
3 changed files with 63 additions and 14 deletions

View file

@ -1311,5 +1311,46 @@ printf -v 'got[1][two][3]' 'ok\f%012d\n' $ver 2>/dev/null
"(expected $(printf %q "$exp"), got $(printf %q "$got"))"
unset got ver
# ======
# The rm builtin's -d option should remove files and empty directories without
# removing non-empty directories (unless the -r option is also passed).
# https://www.austingroupbugs.net/view.php?id=802
if builtin rm 2> /dev/null; then
echo foo > "$tmp/bar"
mkdir "$tmp/emptydir"
mkdir -p "$tmp/nonemptydir1/subfolder"
mkdir "$tmp/nonemptydir2"
echo dummyfile > "$tmp/nonemptydir2/shouldexist"
# Tests for lone -d option
got=$(rm -d "$tmp/emptydir" 2>&1)
[[ $? == 0 ]] || err_exit 'rm builtin fails to remove empty directory with -d option' \
"(got $(printf %q "$got"))"
[[ -d $tmp/emptydir ]] && err_exit 'rm builtin fails to remove empty directory with -d option'
got=$(rm -d $tmp/bar 2>&1)
[[ $? == 0 ]] || err_exit 'rm builtin fails to remove files with -d option' \
"(got $(printf %q "$got"))"
[[ -f $tmp/bar ]] && err_exit 'rm builtin fails to remove files with -d option'
rm -d "$tmp/nonemptydir1" 2> /dev/null
[[ ! -d $tmp/nonemptydir1/subfolder ]] && err_exit 'rm builtin has unwanted recursion with -d option on folder containing folder'
rm -d "$tmp/nonemptydir2" 2> /dev/null
[[ ! -f $tmp/nonemptydir2/shouldexist ]] && err_exit 'rm builtin has unwanted recursion with -d option on folder containing file'
# Recreate non-empty directories in case the above tests failed
mkdir -p "$tmp/nonemptydir1/subfolder"
mkdir -p "$tmp/nonemptydir2"
echo dummyfile > "$tmp/nonemptydir2/shouldexist"
# Tests combining -d with -r
got=$(rm -rd "$tmp/nonemptydir1" 2>&1)
[[ $? == 0 ]] || err_exit 'rm builtin fails to remove non-empty directory and subdirectory with -rd options' \
"(got $(printf %q "$got"))"
[[ -d $tmp/nonemptydir1/subfolder || -d $tmp/nonemptydir1 ]] && err_exit 'rm builtin fails to remove all folders with -rd options'
got=$(rm -rd "$tmp/nonemptydir2" 2>&1)
[[ $? == 0 ]] || err_exit 'rm builtin fails to remove non-empty directory and file with -rd options' \
"(got $(printf %q "$got"))"
[[ -f $tmp/nonemptydir2/shouldexist || -d $tmp/nonemptydir2 ]] && err_exit 'rm builtin fails to remove all folders and files with -rd options'
fi
# ======
exit $((Errors<125?Errors:125))

View file

@ -1,7 +1,7 @@
/***********************************************************************
* *
* This software is part of the ast package *
* Copyright (c) 1992-2012 AT&T Intellectual Property *
* Copyright (c) 1992-2013 AT&T Intellectual Property *
* Copyright (c) 2020-2021 Contributors to ksh 93u+m *
* and is licensed under the *
* Eclipse Public License, Version 1.0 *
@ -15,8 +15,8 @@
* AT&T Research *
* Florham Park NJ *
* *
* Glenn Fowler <gsf@research.att.com> *
* David Korn <dgk@research.att.com> *
* Glenn Fowler <glenn.s.fowler@gmail.com> *
* David Korn <dgkorn@gmail.com> *
* *
***********************************************************************/
#pragma prototyped
@ -28,7 +28,7 @@
*/
static const char usage[] =
"[-?\n@(#)$Id: rm (AT&T Research) 2012-02-14 $\n]"
"[-?\n@(#)$Id: rm (AT&T Research) 2013-12-01 $\n]"
"[--catalog?" ERROR_CATALOG "]"
"[+NAME?rm - remove files]"
"[+DESCRIPTION?\brm\b removes the named \afile\a arguments. By default it"
@ -43,11 +43,9 @@ static const char usage[] =
" writing a 0 filled buffer the same size as the file, executing"
" \bfsync\b(2) and closing before attempting to remove. Implemented"
" only on systems that support \bfsync\b(2).]"
"[d:directory?\bremove\b(3) (or \bunlink\b(2)) directories rather than"
" \brmdir\b(2), and don't require that they be empty before removal."
" The caller requires sufficient privilege, not to mention a strong"
" constitution, to use this option. Even though the directory must"
" not be empty, \brm\b still attempts to empty it before removal.]"
"[d:directory?If the current entry is a directory then remove it using "
"\brmdir\b(3) instead of the default \bunlink\b(2). If \b--recursive\b "
"is not specified then non-empty directories will not be removed.]"
"[f:force?Ignore nonexistent files, ignore no file operands specified,"
" and never prompt the user.]"
"[i:interactive|prompt?Prompt whether to remove each file."
@ -84,7 +82,7 @@ typedef struct State_s /* program state */
{
Shbltin_t* context; /* builtin context */
int clobber; /* clear out file data first */
int directory; /* remove(dir) not rmdir(dir) */
int directory; /* rmdir(dir) not unlink(dir) */
int force; /* force actions */
int interactive; /* prompt for approval */
int recursive; /* remove subtrees too */
@ -151,8 +149,11 @@ rm(State_t* state, register FTSENT* ent)
if (!state->recursive)
{
fts_set(NiL, ent, FTS_SKIP);
error(2, "%s: directory", ent->fts_path);
break;
if (!state->directory)
{
error(2, "%s: directory", ent->fts_path);
break;
}
}
if (!beenhere(ent))
{
@ -188,7 +189,7 @@ rm(State_t* state, register FTSENT* ent)
nonempty(ent);
}
}
if (ent->fts_info == FTS_D)
if (!state->directory && ent->fts_info == FTS_D)
break;
}
else
@ -209,7 +210,7 @@ rm(State_t* state, register FTSENT* ent)
path = ent->fts_accpath;
if (state->verbose)
sfputr(sfstdout, ent->fts_path, '\n');
if ((ent->fts_info == FTS_DC || state->directory) ? remove(path) : rmdir(path))
if ((state->recursive || state->directory) ? rmdir(path) : unlink(path))
switch (errno)
{
case ENOENT:
@ -385,6 +386,8 @@ b_rm(int argc, register char** argv, Shbltin_t* context)
}
if (!*argv)
return 0;
if (state.directory && state.recursive)
state.directory = 0; /* the -r option overrides -d */
/*
* do it