HiveBrain v1.2.0
Get Started
← Back to all entries
patternbashMinor

trash script to alias rm

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
scripttrashalias

Problem

This script implements a command line level trashcan system.

Designed to work around rm doing the bits needed to implement a trashcan system at command line level. Any user flags hands the request over to rm. This means rm -f will still delete a file with no recovery. Prevents execution by root as too many dependencies at superuser level. Deployment by adding aliases rm=trash and rmoops=untrash. Only implemented on files, does not cater for directories.

Appreciate any and all suggestions from formatting, expansion of the idea to glaring issues. Looking for an experienced eye to pin point potential issues.

/usr/local/bin/trash

#!/bin/bash
#
# Trashcan script
# alias rm=trash
#

error() {
echo $1
exit 1
}

bypass() {
exec rm $*
}

file="$1"
trash=~/.trash
keep=5

## bypass scenarios
# root user
[ "$(id -u)" = "0" ] && error "Please do not run as root user."
# special cases
case $file in
-) bypass $ ;;
$trash/ ) bypass $ ;;
esac
# only single filename supported
[ $# -gt 1 ] && bypass $*
# can we access the file?
[ -f "$file" ] || error "File not specified."

# move existing versions out of the way
ls $trash/$file.version.* 2>/dev/null | tac | while read name; do
version=$((${name##*.}+1))
# keep the only the last number of versions
if [ $version -gt $keep ]; then
rm $name || error "Unable to remove file $name."
else
mv $name $trash/$file.version.$version || error "Unable to move version $version."
fi
done

# move file into trash (volume friendly)
cp --preserve $file $trash/$file.version.1 || error "Unable to trash your file."
rm -f $file || error "Unable to cleanup the file."

exit 0


/usr/local/bin/untrash

`#!/bin/bash
#
# Restore files from Trashcan
# alias rmoops=untrash
#

error() {
echo $1
exit 1
}

file="$1"
trash=~/.trash
keep=5

## bypass scenarios
# root user
[ "$(id -u)" = "0" ] && error "Please do not run as root user."

# any flags so rm -rf actually does it
case $file in
-*) error "Flags are not supp

Solution

Overall

I'm not really convinced of the value of this "trashcan" idea. If deleting files doesn't actually delete them, then what's the point? Trying to create free space would be ineffectual. Worse, encouraging users to lean on an "undelete" facility will likely leave them in trouble when they move to a conventional system without such a crutch. (Plus, it's a Good Thing to have an occasional minor accident - otherwise it's very easy to just assume that your backups are working, and never think to verify that you can actually restore from them! I'm certainly guilty of that...)
Quoting issues

Much of your code doesn't quote expanded variables when it should. One example:

case $file in
  -*) bypass $* ;;
  $trash/* ) bypass $* ;;
esac


This is unsafe, and needs to be written

case "$file" in
  -*) bypass "$@" ;;
  "$trash"/* ) bypass "$@" ;;
esac


There are many more similar instances; I won't list them all.
Testing for file-ness rather than existence

[ -f "$file" ] || error "File not specified."


Do you really want this error when you ask to trash a symlink? In fact, I think the wording of the message is poor, even for the case you think you're testing.
Pathname matching

You test whether the argument is in the "trash" directory with

case $file in
  $trash/* )
esac


There's other ways to refer to the same directory as "$trash" - in particular, a relative name from the working directory. You'll need to canonicalise path name if you're doing this kind of comparison; possibly you might want to compare device and inode numbers for real robustness if you might have bind mounts and the like.
Echo and read

This code:

echo -n "Which would you like to restore? [1]:"
read user


is usually written

read -p "Which would you like to restore? [1]: " user


(I'm not sure that user is a good name for the version number, either).

Code Snippets

case $file in
  -*) bypass $* ;;
  $trash/* ) bypass $* ;;
esac
case "$file" in
  -*) bypass "$@" ;;
  "$trash"/* ) bypass "$@" ;;
esac
[ -f "$file" ] || error "File not specified."
case $file in
  $trash/* )
esac
echo -n "Which would you like to restore? [1]:"
read user

Context

StackExchange Code Review Q#157299, answer score: 2

Revisions (0)

No revisions yet.