[ale] Anyone want to crtique a shell script?

[Alex Carver]

First, a lot of your statements are missing semicolons.  You'll need
those or bash will complain.

Your exit for any unknown option should use a non-zero exit value.  Just
add a value to the exit command:

echo "Sorry, I don't understand $OPTARG."
exit 1

You can choose most values below 127.  If you follow the way bash does
it for built-in functions, they use exit code 2 for improper usage.

On lines 61-64 it looks like you're trying to null out the file using a
redirect but you don't use an input.  You may want to either explicitly
cat /dev/null into the file or do an rm followed by a touch that way you
are less likely to get any nasty surprises (like stdin suddenly showing
up in OUTFILE).

cat /dev/null > $OUTFILE
rm $OUTFILE && touch $OUTFILE
	

In the big for loop you could clean up a bit by using a temporary
variable and then moving the -z check to a single statement (plus add a
condition for a missing file which you don't have) and have only a
single pair of cat statements:

for file in install network services partitions packages pre post;
do
	if [ -f "${file}.${RELEASE}.${SUFFIX}" ];
		INFILE=${file}.${RELEASE}.${SUFFIX};
	elif [ -f "${file}.${RELEASE}" ]
		INFILE=${file}.${RELEASE};
	elif [ -f "${file}" ]
		INFILE=${file};
	else
		#whoops, there was supposed to be a file here
		break; #get out of the loop before something blow up
		#alternatively abort with an error code
		#exit 10
	fi;

	if [ -z $OUTFILE ];
		cat "${INFILE}";
	else
		cat "${INFILE}" >> $OUTFILE;
	fi;
done

Lastly if you're worried about paths then append pwd to the outfile
before using it instead of the ./ and also use basename to strip any
paths (so someone can't be clever and use lots of ../../.. to grab a
file they shouldn't)

OUTFILE=`basename "${OUTFILE}"`
OUTFILE="${PWD}/${OUTFILE}"

On 2015-04-11 07:22, Leam Hall wrote:
> Could use critique on a shell script that writes Kickstart files. You
> don't have to understand Kickstart to critique, the output files are
> plain text in a certain order.
> 
> https://github.com/LeamHall/SecComFrame/tree/master/tools/ks_writer
> 
> The script is ks_writer.sh and the other files there are what gets
> pulled in and what an output might look like.
> 
> Thanks!
> 
> Leam
> _______________________________________________
> Ale mailing list
> Ale at ale.org
> http://mail.ale.org/mailman/listinfo/ale
> See JOBS, ANNOUNCE and SCHOOLS lists at
> http://mail.ale.org/mailman/listinfo
> 
>