4
\$\begingroup\$
#!/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%s033円[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
finnw
7771 gold badge5 silver badges9 bronze badges
asked Oct 10, 2011 at 4:36
\$\endgroup\$

3 Answers 3

2
\$\begingroup\$

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! :)

answered Oct 12, 2011 at 14:43
\$\endgroup\$
5
  • \$\begingroup\$ Why is [] better than test? (The majority of projects that I've worked on prefer test to [] so I'm interested in your rationale.) \$\endgroup\$ Commented Oct 12, 2011 at 20:02
  • \$\begingroup\$ @CharlesBailey, Functionally, [ ] is the same as test, so in that regard it doesn't matter. Many find [ ] is easier to read than test. But, and this is important, neither compare to the power of [[ ]] which should be regularly used in favor of both [ ] and test. Check out the documentation on conditional constructs for more info. (Notice that [ ] and test aren't even mentioned any more! They're deprecated!) \$\endgroup\$ Commented Oct 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 to test and [] being deprecated, I am very surprised to here that. \$\endgroup\$ Commented 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\$ Commented Oct 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\$ Commented Oct 12, 2011 at 21:11
1
\$\begingroup\$

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 variables

  • here-strings are not portable

answered Nov 13, 2011 at 12:56
\$\endgroup\$
1
\$\begingroup\$

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.

answered May 14, 2016 at 6:12
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.