Librarian script to find and copy a file

Question

I wrote this script partly as an exercise in learning Bash and partly because I needed a tool to find specific versions of library files and copy them to a particular device.

The script takes parameters that control its operation. First, it searches for the file specified by the user. Then it displays the search results, allowing the user to pick one of the files found, and (assuming a file was selected) copying it to the specified destination/path.

I would most like feedback on whether the script could be written more clearly or perhaps whether there are better ways to approach its tasks.

#!/bin/bash
# libr.sh (librarian): find the file you need, and put it where you want.
#-------------------------process parameters
if [ $# -eq 0 ] || [ $# -gt 3 ]
then
 echo "Usage: libr <FILENAME> [DESTINATION] [IP]"
 echo "The librarian will find FILENAME in the current directory or below"
 echo "and will then copy it to the DESTINATION path on the unit at IP."
 echo "If multiple files are found, the user may choose which to copy."
 exit
fi
FILE=1ドル
DEST=${2:-/usr/lib/}
IP=${3:-160.48.199.99}
#-------------------------find the file
FULLRESULTS=$(find -name $FILE 2> /dev/null)
echo "File \"$FILE\" was found in the following directories:"
if [ -z "$FULLRESULTS" ]
then
 echo " None!"
 exit
fi
# `find` returns the path plus filename for each file found
# but we just want the directories, for ease of reading
RESULTS=$(dirname $FULLRESULTS)
#-------------------------display results and get the source path
PS3="Which one do you want to copy ? "
select SRCPATH in $RESULTS
do
 if [ ! -z "$SRCPATH" ]
 then
 #-----------------copy the file
 CMD="scp $SRCPATH/$FILE root@$IP:$DEST"
 echo "Executing: $CMD"
 $CMD
 fi
 # we don't actually want to loop
 break
done

Answer 1

Thanks for submitting this for a code review. I like the indentation and documentation in your code.

The biggest problem I see with this is dealing with filenames that haves spaces in them. Your select line will almost certainly break. The find command will let you get the results null-terminated using -print0, but I'm not sure how to get the select to work with that. Hopefully in your use-case you can avoid filename with spaces and avoid fixing this.

Small suggestions

• Mention the defaults in the usage printout. Maybe move the defaults to variables so you Don't Repeat Yourself.
• Use double square brackets around conditionals to avoid unpleasant surprises and subtle bugs.
• Move the thens up on the same line as the if with ; then.
• Consider checking for whether the find got any results before printing "was found". You could print out one message for no results and only print your current "File ... was found ..." message if something was actually found.
• Rather than put the scp command into a variable just so you can display it you could cut on tracing with set -x and then cut it back off right after with set +x.
• Check out shellcheck.