#pragma section-numbers 2

= Shell Hall of Shame =

These pages show actual scripts, written and used in the "real world", which have potentially serious mistakes in them.  The reader is expected to understand the common [[BashPitfalls|pitfalls]] associated with writing shell scripts.

The scripts herein may be copyrighted by various people or companies.  I don't care.  If you feel you need to protect your "rights" on utter crap, I feel rather sorry for you.

In my experience, commercial Unix flavors tend to have rather poorly written shell scripts.  My theory for why this occurs is that shell scripting is usually done at the last minute, when someone needs a little tweak here or there, or needs a simple job done, or to maintain backward compatibility when a complex application has changed.  As such, I believe that companies assign scripting jobs to junior programmers, and reserve the labor of their most experienced developers for the complex applications and tools.  These junior programmers don't always have the experience or knowledge to see all their mistakes.  And then, the senior programmers, who've been working on databases or kernels or whatever, either haven't learned the syntax of shell scripting in depth, or aren't brought in for code reviews on the scripts.

In any case, whether my theory is correct or not, the resulting products are often quite seriously flawed.  Here are some examples.

== /usr/bin/lpr (HP-UX 11.11) ==
{{{#!highlight sh
#!/usr/bin/sh
# @(#)B.11.11_LR     
# lpr script -- which makes lp look like lpr

options=
files=
remove=false

while [ $# -gt 0 ]
do
	case $1 in
		-c)
			options="$options $1";;
		-d*)
			options="$options $1";;
		-m)
			options="$options $1";;
		-n*)
			options="$options $1";;
		-o*)
			options="$options $1";;
		-r)
			remove=true;;
		-s)
			options="$options $1";;
		-t*)
			options="$options $1";;
		-w)
			options="$options $1";;
		-*)
			echo "$0: option $1 not recognized"
			exit 1;;
		*)
			files="$files $1";;
	esac
	shift
done

cat $files | lp -s $options

if [ $remove = true ]
then
	rm -f $files
fi
}}}

This is an example of a [[WrapperScript|wrapper script]].  Stylistically, this looks pretty good.  There's just enough commenting to show what we're doing, and the variables have short but easily understandable names.  The indenting is correct (if a bit overzealous).  I'm not personally fond of the extraneous whitespace after each case label, but it's not a bad thing.

The problem here is ''what happens when a filename contains whitespace''.  Instead of being handled and passed along individually, all the filenames are being concatenated together into a single string, with a single space between them.  And then the shell's [[WordSplitting|word splitting]] is used to divide the filenames apart.  If a filename already contains a space (or tab, newline, etc.), the resulting list of names given to `cat` on line 39 will not match the original filenames given to the wrapper script.

A similar problem might occur if any of the other options contains a space.  The `options` variable builds up a list of them, and then (again on line 39), word splitting is used to break them into a list, for the `lp` command.  If any options are given which contain whitespace, they will be split apart.  (This may or may not be within the acceptable range of input for `lpr` or `lp` options, so this part ''might'' not actually be a problem.  But I wouldn't want to take that chance.)

There is, in general, no clean way to handle this problem in the classic [[BourneShell|Bourne shell]].  But HP wasn't limited to the use of the Bourne shell when they wrote this program.  It's part of a commercial product -- a Unix derivative that runs only on their own hardware.  They don't need it to be portable to other platforms.  They could use any of the features of their own shells, and this includes both POSIX and [[KornShell|Korn shell]] (ksh88), which are shipped with the operating system.  ksh88's [[BashFAQ/005|arrays]] are good enough to be used for this purpose:

{{{
#!/usr/bin/ksh
# ksh88
unset files i
...
case $1 in
   ...
   *) files[$i]="$1"; i=$(($i+1));;
esac
...
cat "${files[@]}" | lp ...
}}}

That would have avoided any problems with filenames containing whitespace.  A similar bit of code could be used to build up an array of the options to be passed to `lp`.

[[/Page2|Page 2: /usr/dt/bin/Xsession ->]]

----
CategoryShell CategoryUnix