#!/bin/bash
# Used to add/remove printers from system (lp)
# Copyright Fresh Computer Systems Pty Ltd.
GREP_OPTIONS='--color=tty'
export GREP_OPTIONS
sanity() {
# Are we root?
if [ $EUID != '0' ]; then
exitError "You must be root to run this script." 1
fi
if [ ! -x /usr/sbin/lpadmin -a ! -x /sbin/lpadmin ]; then
# debug
#printf " @@@ ERROR: \'lpadmin\' was not found, or is not executable. Quitting.\n\n"
exitError " @@@ ERROR: \'lpadmin\' was not found, or is not executable" 1
fi
PPD="/usr/share/cups/model/postscript.ppd"
PPDGZ="/usr/share/cups/model/postscript.ppd.gz"
if [ -f $PPD ]; then
POSTSCRIPT="$PPD"
elif [ -f $PPDGZ ]; then
POSTSCRIPT="$PPDGZ"
else
exitError " @@@ ERROR: No postscript file found. Please ensure there is a \"$PPD\" or a \"$PPDGZ\" file." 1
fi
# ensure sbin is in path
PATH=/usr/sbin:/sbin:${PATH}
# /etc/hosts file
HOSTS="/etc/hosts"
if [ ! -w "$HOSTS" ]; then
exitError "Hosts file \"$HOSTS\" is not writeable." 1
fi
}
exitError() {
# Provide default exit code '1' if none provided
if [ -n "$1" ]; then
ERRMSG="$1"
ERRCODE="${2-1}"
else
ERRMSG=" @@@ ERROR: Unspecified Error."
ERRCODE="${1-1}"
fi
ERRMSG="$ERRMSG"
printf "\n%s\n" "$ERRMSG" >&2
printf "%s\n\n" "Quitting." >&2
exit ${ERRCODE}
}
invalidInput() {
declare USELESS
printf "\nInvalid input.\n" >&2
read -p "Press Any Key To Continue..." USELESS
#unset USELESS
printf "\n"
}
##
## FUNCTIONS
##
enterPrinterName() {
unset PRINTER_TYPE PRINTER_NAME IPADDRESS IPADDRESS_SED CONNECTION_TYPE REPLY RESULT PORTNAME NAME_EXISTS_IN_HOSTS ADDRESS_EXISTS_IN_HOSTS HOSTACTION RC
printf "\n"
while true; do
read -p "Enter printer name, or [q]uit: " PRINTER_NAME
case $PRINTER_NAME in
[qQ]) exitError "Aborted by User" 0
;;
??*) break
;;
""|*) invalidInput #blank line or anything else
;;
esac
done
}
checkPrinterExistsInSystem () {
if lpstat -v $PRINTER_NAME 1>/dev/null 2>&1; then
# Printer already exists in the system
#lpstat -v $PRINTER_NAME
#printf "Printer already exists in system!\n" >&2
declare EXISTS=0 #true
else
declare EXISTS=1 #false
fi
printf "\n"
NAME_EXISTS_IN_SYSTEM=$EXISTS
return $EXISTS
}
checkNameExistsInHosts() {
declare EXISTS=1
# scan each line from $HOSTS
while read LINE; do
# Remove comments
LINE="${LINE%%#*}"
case "$LINE" in
*${PRINTER_NAME}*)
# printer found
EXISTS=0
break #stop looping
;;
*)
# printer not found
:
;;
esac
done < $HOSTS
if [ $EXISTS -eq 0 ]; then
printf "Printer \"$PRINTER_NAME\" already exists in \"$HOSTS\"!\n\n" >&2
egrep "^[^#]*$PRINTER_NAME[[:space:]]*" "${HOSTS}"
fi
printf "\n"
NAME_EXISTS_IN_HOSTS=$EXISTS
return $EXISTS
}
checkAddressExistsInHosts() {
declare EXISTS=1
while read LINE; do
# Remove comments
LINE="${LINE%%#*}"
case "$LINE" in
*${IPADDRESS}*)
# address exists in hosts
EXISTS=0 #true
break
;;
*)
# address does not exist
:
;;
esac
done < $HOSTS
if [ $EXISTS -eq 0 ]; then
# address exists, add alias
#echo "............. HERE I AM 1 .............. "
HOSTACTION=addAliasToHosts
printf "IP address \"$IPADDRESS\" already exists in \"$HOSTS\"!\n\n"
egrep "^[^#]*$IPADDRESS[[:space:]]*" "${HOSTS}"
else
# address is NEW, add as a new host
#echo "............. HERE I AM 2 .............. "
HOSTACTION=addHostToHosts
fi
ADDRESS_EXISTS_IN_HOSTS=$EXISTS
printf "\n"
return $EXISTS
}
getConnectionType() {
while true; do
# Connection type
printf "\nCONNECTION TYPE:
1. LPD (print server)
2. 9100 (network printer)\n\n"
read -p "Enter a number, or [q]uit: " REPLY
case $REPLY in
[qQ]) exitError "Aborted by User." 0
;;
1) CONNECTION_TYPE="LPD"
break
;;
2) CONNECTION_TYPE=9100
break
;;
""|*) invalidInput
;;
esac
done
printf "Connection type selected: \"$CONNECTION_TYPE\".\n\n"
if [ "$CONNECTION_TYPE" = "LPD" ]; then
# LPD connection requires a portname
while true; do
read -p "$CONNECTION_TYPE: give portname (e.g. p1), or [q]uit: " REPLY
case $REPLY in
[qQ]) exitError "Aborted by User." 0
;;
[[:alnum:]]*) PORTNAME="$REPLY" #at least one alphanumeric char
break
;;
""|*) invalidInput
;;
esac
done
printf "OK: Portname selected: \"$PORTNAME\".\n\n"
fi
}
getPrinterType() {
while true; do
# Printer Type
read -p "Is it a POSTSCRIPT printer, or [q]uit? [y/n/q] "
case $REPLY in
[qQ]) exitError "Aborted by User." 0
;;
[yY]) PRINTER_TYPE="postscript"
break
;;
[nN]) PRINTER_TYPE="raw"
break
;;
""|*) invalidInput
;;
esac
done
printf "OK: Printer type selected: \"$PRINTER_TYPE\".\n\n"
}
getIpAddr() {
while true; do
read -p "Enter full IP address (e.g. 192.168.111.222) of printer \"$PRINTER_NAME\", or [q]uit: " REPLY
case $REPLY in
[qQ]) exitError "Aborted by User." 0
;;
*.*.*.*) IPADDRESS="$REPLY" #no robust pattern checking...
break
;;
""|*) invalidInput
;;
esac
done
printf "OK: IP address selected: %s\n\n" "$IPADDRESS"
}
addHostToHosts() {
if ! [ -w "$HOSTS" -a -n "$IPADDRESS" -a -n "$PRINTER_NAME" ]; then
# debug
#printf "\$HOSTS = $HOSTS\n"
#printf "\$IPADDRESS = $IPADDRESS\n"
#printf "\$PRINTER_NAME = $PRINTER_NAME\n"
exitError " @@@ ERROR: Error in function \"$FUNCNAME\" on line \"$LINENO\"." 1
fi
# append host to bottom of $HOSTS
printf "%-20s%s\n" "$IPADDRESS" "$PRINTER_NAME" >> "${HOSTS}"
# show result
egrep "\<$IPADDRESS\>" "$HOSTS"
ERROR=$?
return $ERROR
}
addAliasToHosts() {
if ! [ -w "${HOSTS}" -a -n "$IPADDRESS" -a -n "$PRINTER_NAME" ]; then
exitError " @@@ ERROR: Error in function \"$FUNCNAME\" on line \"$LINENO\"." 1
fi
# get IPADDRESS line in $HOSTS
declare PRINTER_NAME_LINE=$(egrep "^[^#]*\<$IPADDRESS\>" "$HOSTS")
########
#while read LINE; do
# # Remove comments
# LINE="${LINE%%#*}"
# case "$LINE" in
# *${IPADDRESS}*)
# # address found
# declare EXISTS=0
# break #stop looping
# ;;
# *)
# # address not found
# declare EXISTS=1
# ;;
# esac
#done < $HOSTS
########
# store data (aka remove comments)
declare DATASTORE=${PRINTER_NAME_LINE%%#*}
# append new printer name to ip address
declare NEW_PRINTER_NAME_LINE="$DATASTORE $PRINTER_NAME"
# store comments (aka remove data)
case $PRINTER_NAME_LINE in
*#*) declare COMMENTSTORE=${PRINTER_NAME_LINE#*#}
# append alias to stored data
declare NEW_PRINTER_NAME_LINE="$NEW_PRINTER_NAME_LINE#$COMMENTSTORE"
;;
esac
# append alias to $HOSTS
printf "Appending \"$PRINTER_NAME\" to \"$HOSTS\" as alias using IP address \"$IPADDRESS\".\n\n"
sed -r -e "s/$PRINTER_NAME_LINE/$NEW_PRINTER_NAME_LINE/" -i $HOSTS
RESULT=$?
# show result
egrep "\<$IPADDRESS\>" "$HOSTS"
return $RESULT
}
removeFromHosts() {
# grab line with printer name
declare PRINTER_NAME_LINE=$(egrep "^[^#]*$PRINTER_NAME[[:space:]]*" ${HOSTS})
#printf "\$PRINTER_NAME_LINE = $PRINTER_NAME_LINE\n"
case "$PRINTER_NAME_LINE" in
*#*) # save comment for later (aka delete data)
declare COMMENTSTORE="${PRINTER_NAME_LINE#*#}"
declare HASCOMMENT=0 #true
;;
*) declare HASCOMMENT=1 #false
;;
esac
# store data (aka remove all comments)
declare DATASTORE="${PRINTER_NAME_LINE%%#*}"
#printf "\$DATASTORE = $DATASTORE\n"
# delete PRINTER_NAME
declare DATASTORE=${DATASTORE/$PRINTER_NAME}
#printf "\$DATASTORE = $DATASTORE\n"
backupHosts || exit
# Do we have a host remaining for this IP address?
# REGEX: ipaddress followed by one or more spaces, followed by one or more alphanumeric chars
if egrep "^[^#]*[[:digit:]]{0,3}\.[[:digit:]]{0,3}\.[[:digit:]]{0,3}\.[[:digit:]]{0,3}[[:space:]]+[[:alnum:]]+" <<<"${DATASTORE}" >/dev/null 2>&1; then
# we still have a hostname/alias
if [ $HASCOMMENT -eq 0 ]; then
# Comment present. Append it.
declare DATASTORE="${DATASTORE}#${COMMENTSTORE}"
fi
# BUG: TODO: this is erasing ALL occurences of printer_name
# should only modify the one we want
#sed -r -e "s/$PRINTER_NAME_LINE[[:space:]]*/$NEW_PRINTER_NAME_LINE/g" -i "$HOSTS"
sed -r -e "s/^[^#]*$PRINTER_NAME_LINE[[:space:]]*/$DATASTORE/g" -i "${HOSTS}"
SED_RESULT=$?
else
# we have no hostnames/aliases remaining. Delete entire line
#printf " ... running sed ... \n"
#printf "\$PRINTER_NAME_LINE = $PRINTER_NAME_LINE\n"
#printf "\$DATASTORE = $DATASTORE\n"
sed -r -e "/^[^#]*$PRINTER_NAME/d" -i "$HOSTS"
SED_RESULT=$?
fi
if [ $SED_RESULT -eq 0 ]; then
printf "Ok: Printer \"$PRINTER_NAME\" successfully removed from \"$HOSTS\".\n"
else
exitError " @@@ ERROR: Fatal error on line \"$LINENO\" in function \"$FUNCNAME\"." 1
fi
return $SED_RESULT
#printf "...reached end of removeFromHosts...\n"
}
backupHosts () {
printf "Making backup copy of \"$HOSTS\" to \"${HOSTS}.$$\".\n"
#printf "cp -v \"${HOSTS}\" \"${HOSTS}.$$\"\n\n"
cp "${HOSTS}" "${HOSTS}.$$"
RESULT=$?
if [ $RESULT -eq 0 -a -f "$HOSTS.$$" ]; then
printf "Successfully backed up hosts file.\n\n"
else
printf "Backup of hosts file was not successful.\n\n" >&2
fi
return $RESULT
}
lpadd() {
COMMANDLINE="lpadmin -p ${PRINTER_NAME} -E"
if [ -n "${PRINTER_NAME}" -a -n "${PRINTER_TYPE}" -a -n "${CONNECTION_TYPE}" ]; then
# PRINTER_TYPE
if [ "${PRINTER_TYPE}" = "raw" ]; then
# Printer Type: RAW
COMMANDLINE="${COMMANDLINE} -m raw"
elif [ "$PRINTER_TYPE" = "postscript" ]; then
# Printer type: postscript
COMMANDLINE="${COMMANDLINE} -P $POSTSCRIPT"
else
exitError " @@@ ERROR: Invalid Printer Type: \"$PRINTER_TYPE\"." 1
fi
# CONNECTION_TYPE
if [ "$CONNECTION_TYPE" = "LPD" ]; then
if [ -n "${PORTNAME}" ]; then
COMMANDLINE="${COMMANDLINE} -v lpd://${PRINTER_NAME}/${PORTNAME}"
else
exitError "Error: Missing \$PORTNAME." 1
fi
elif [ "$CONNECTION_TYPE" = 9100 ]; then
COMMANDLINE="${COMMANDLINE} -v socket://${PRINTER_NAME}:9100"
else
exitError " @@@ ERROR: Invalid Connection Type: \"$CONNECTION_TYPE\"." 1
fi
else
# debug
echo " -------------------------- *** ---------------------------"
echo "----------------- SHOULD NEVER BE HERE ------------------"
echo " -------------------------- *** ---------------------------"
exitError " @@@ ERROR: Serious error. Insufficient values provided. Function \"$FUNCNAME.\"" 1
# debug
printf "printer name: %s\nprinter type: %s\nconnection type: %s\nportname: %s\n" \
"$PRINTER_NAME" "$PRINTER_TYPE" "$CONNECTION_TYPE" "$PORTNAME"
fi
# provide error policy
COMMANDLINE="${COMMANDLINE} -o printer-error-policy=retry-job"
while true; do
printf "About to run this command: \033[1;31m%s\033[0m\n\n" "${COMMANDLINE}"
read -p "Are you sure? 'yes' or [q]uit? [yes/q] " REPLY
case $REPLY in
[qQ]) exitError "Aborted by User." 0
;;
yes|YES) break #yes. doAddPrinter
;;
""|*) invalidInput
;;
esac
done
}
doAddPrinter() {
declare SUCCESS
# backup hosts file
backupHosts || exit
#if [ "$ADDTOHOSTS" = "alias" ] ; then
# addAliasToHosts
#else
# addHostToHosts
#fi
# add host, or alias
# debug
#printf "\$HOSTACTION = $HOSTACTION\n"
eval $HOSTACTION
# add printer
printf "\n"
eval $COMMANDLINE
#if eval $COMMANDLINE ; then
if checkPrinterExistsInSystem; then
SUCCESS=0
lpstat -v "$PRINTER_NAME"
printf "Printer successfully added.\n\n"
else
SUCCESS=1
printf "Printer was not successfully added!\n\n" >&2
#exit
fi
return $SUCCESS
}
acceptPrinter () {
printf "Enabling new printer \"$PRINTER_NAME\"...\n"
if which accept >/dev/null 2>&1; then
accept $PRINTER_NAME
elif which cupsenable >/dev/null 2>&1; then
cupsenable $PRINTER_NAME
else
printf "Could not enable printer. Try running 'accept $PRINTER_NAME' or 'cupsenable $PRINTER_NAME' manually.\n" >&2
false
fi
RC=$?
return $RC
}
lprm() {
while true; do
read -p "Are you sure you want to remove printer \"$PRINTER_NAME\"? [yes/q]: " REPLY
case $REPLY in
[qQ]) exitError "Aborted by User." 0
;;
yes|YES) break
;;
""|*) invalidInput
;;
esac
done
lpadmin -x "${PRINTER_NAME}" # delete printer
if checkPrinterExistsInSystem; then # does printer still exist?
# printer remains -- bad
declare SUCCESS=1 #false
else
# printer deleted -- good
declare SUCCESS=0 #true
#FIXME
removeFromHosts
fi
return $SUCCESS
}
main () {
enterPrinterName
if checkPrinterExistsInSystem; then
lpstat -v ${PRINTER_NAME}
printf "\nPrinter \"${PRINTER_NAME}\" already exists in the system.\n"
while true; do
read -p "[d]elete printer, [r]estart, or [q]uit? [r/s/q]? " REPLY
case $REPLY in
[qQ]) exitError "Aborted by User." 0
;;
[dD]) if lprm "$PRINTER_NAME"; then
printf "OK: Printer \"${PRINTER_NAME}\" successfully removed from the system.\n\n"
#break #call checkNameExistsInHosts
return 0
else
exitError " @@@ ERROR: Failure to remove printer from the system.\n" 1
fi
;;
[rR]) return 0 #exit main, start loop again, restart script
;;
esac
done
else
# consider deleting completely
: #printf "OK: Printer \"${PRINTER_NAME}\" does not exist in the system.\n\n"
fi
if checkNameExistsInHosts; then
while true; do
read -p "Are the details correct [y], [r]estart or [q]uit? [y/r/q] " REPLY
case $REPLY in
[qQ]) exitError "Aborted by User." 0
;;
[yY]) #if ok -> getData()
break
;;
[rR]) #if not ok -> loop back main
printf "OK: Restarting script.\n\n"
return 0 #quit main and restart script
;;
""|*) invalidInput
;;
esac
done
else
#if name does not exist in hosts, get ip addr
getIpAddr
if checkAddressExistsInHosts; then
while true; do
read -p "Add as an alias to \"$HOSTS\", or [q]uit? [y/n/q] " REPLY
case $REPLY in
[qQnN]) exitError "Aborted by User." 0
;;
[yY]) break #addAliasToHosts
;;
""|*) invalidInput
;;
esac
done
fi
fi
getConnectionType
getPrinterType
lpadd
doAddPrinter
acceptPrinter
}
##
## MAIN PROGRAM
##
sanity
while true; do
main
done
unset GREP_OPTIONS
3 Answers
To answer your question, yes, it can be improved, but that could be said about any piece of code, right? And perfection is in the eye of the observer, so I'll stick to the things that have proved gotchas for me, and those focus around conditional constructs.
As was mentioned in the comments on your original post on SO, [ ]
is better than test
. But we can go one step further. Some times you'll see scripts that have something similar to this.
if [ x"$VAR" = x"" ]; then ...; fi
This was to avoid a situation where an empty variable could cause a syntax error. Using [[ ]]
automatically avoids this an allows you to write more cleanly.
if [[ $VAR == "" ]]; then ...; fi
This also allows you to have more natural logic operators in your conditional.
if [[ $VAR1 == "yep" && $VAR2 == "sure" ]]; then ...; fi
There are many other cool things that this buys you too. You can now do regular expression comparisons inline.
if [[ $IP =~ ^[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+$ ]]; then echo "valid ip"; fi;
The last thing I'll leave you with the (( ))
. This is very very handy when you know you are operating on numbers. It allows you, among many other things, to compare numbers as numbers rather than strings (as with [ ]
or [[ ]]
).
if (( $ZERO == 0 )); then echo "yep, it's zero"; fi
I'll leave you to explore the recent version of the manual for more cool stuff! :)
-
\$\begingroup\$ Why is
[]
better thantest
? (The majority of projects that I've worked on prefertest
to[]
so I'm interested in your rationale.) \$\endgroup\$ Oct 12, 2011 at 20:02 -
\$\begingroup\$ @CharlesBailey, Functionally,
[ ]
is the same astest
, so in that regard it doesn't matter. Many find[ ]
is easier to read thantest
. But, and this is important, neither compare to the power of[[ ]]
which should be regularly used in favor of both[ ]
andtest
. Check out the documentation on conditional constructs for more info. (Notice that[ ]
andtest
aren't even mentioned any more! They're deprecated!) \$\endgroup\$– perdenOct 12, 2011 at 20:24 -
2\$\begingroup\$
[[ ... ]]
isn't in the POSIX standard, though, so you immediately lose compatibility with standard Bourne shells. Can you provide a reference totest
and[]
being deprecated, I am very surprised to here that. \$\endgroup\$ Oct 12, 2011 at 20:29 -
\$\begingroup\$ @CharlesBailey, The latest version of the Bash manual doesn't reference to the use of
[ ]
and only details[[ ]]
and since[[ ]]
encompass the functionality of[ ]
, that's deprecation by definition! :) Also note: (1)[[ ]]
was introduced near its present capacity circa Bash-2.0, but has since developed. (2)[[ ]]
is not in POSIX, but the standard does mention that these characters are used as reserved words in some implementations (i.e. Bash). (3) The original question is about Bash, so it's worth mentioning[[ ]]
as a good feature to use! :) \$\endgroup\$– perdenOct 12, 2011 at 21:03 -
2\$\begingroup\$
test
and[
are referenced here: gnu.org/s/bash/manual/bash.html#Shell-Builtin-Commands . They are not marked as deprecated and as bash "intended to be a conformant implementation of the ieee posix Shell and Tools portion of the ieee posix specification", nor should they be. I think you were misusing the word "deprecated". I always think it's good to warn about non-portable constructs even when they have useful advantages. \$\endgroup\$ Oct 12, 2011 at 21:11
I see only a couple of minor issues.
I suggest to flip the parameters of exitError
: make the exit code the first and the message the second. As the message tends to get long, the exit code parameter is easy to overlook, which can lead to accidental misuses.
No need to quote literal values like '0'
here, you can write simply 0
:
if [ $EUID != '0' ]; then
Instead of declaring a "useless" variable here:
declare USELESS printf "\nInvalid input.\n" >&2 read -p "Press Any Key To Continue..." USELESS #unset USELESS printf "\n"
You could just not declare anything at all:
printf "\nInvalid input.\n" >&2
read -p "Press Any Key To Continue..."
printf "\n"
The handling of printer name input is a bit odd:
read -p "Enter printer name, or [q]uit: " PRINTER_NAME case $PRINTER_NAME in [qQ]) exitError "Aborted by User" 0 ;; ??*) break ;; ""|*) invalidInput #blank line or anything else ;;
An invalid value is a blank or a single-letter name other than "q" or "Q". If it's important that the name should not be a single-letter, then it would be better to mention that in the prompt. If it's not so important, then the switch can be simplified a bit:
read -p "Enter printer name, or [q]uit: " PRINTER_NAME case $PRINTER_NAME in [qQ]) exitError "Aborted by User" 0 ;; "") invalidInput ;; *) break ;;
This kind of case
statement appears at many places:
""|*) invalidInput
When *
is one of the values, all other values are unnecessary.
Since the script uses /bin/bash
, you could simplify >/dev/null 2>&1
as &>/dev/null
.
I'm not a huge fan of flag variables.
For example in doAddPrinter
you have this:
declare SUCCESS # ... (many many lines) if checkPrinterExistsInSystem; then SUCCESS=0 lpstat -v "$PRINTER_NAME" printf "Printer successfully added.\n\n" else SUCCESS=1 printf "Printer was not successfully added!\n\n" >&2 #exit fi return $SUCCESS
I suggest to not declare SUCCESS
at the top and return
at the end,
but to return in the branches of the final conditional:
if checkPrinterExistsInSystem; then
lpstat -v "$PRINTER_NAME"
printf "Printer successfully added.\n\n"
return 0
else
printf "Printer was not successfully added!\n\n" >&2
return 1
fi
If you really want to use the SUCCESS
variable, then at least declare it right before it's used, so the reader doesn't need to scroll up to verify the declare
or local
keywords.
Some other minor notes on portability (I know that from your shebang you are using bash but a portable script is usually better :-)
declare
is not portable (you could use subshells( ... )
or$( ... )
to define local variableshere-strings are not portable