[Sugar-devel] Quoting in shell scripts (was: Re: [sugar-devel] Recent fiddlings with Print Support)

[Sascha Silbe]

On Wed, May 13, 2009 at 01:03:26PM +0530, Vamsi Krishna Davuluri wrote:

> Okay, so here's the latest dope.
I hope you don't mind me pointing out a few oversights in your script 
publically. The main reason is that I want to remember others (e.g. GSoC 
students) to be careful about quoting - a topic that unfortunately 
doesn't seem to get as much attention in university courses as it 
deserves.

> sandbox=${TMPDIR-/tmp}/cups-odftops.$$
> (umask 077 && mkdir $sandbox) || exit 1
TMPDIR and thus later sandbox may contain any character, so you need to 
quote them.
BTW: I usually issue "set -e" in front of any script and explicitly 
handle the cases where I know that some command may fail and I _do_ want 
to continue, BTW. Doing it the other way round increases the likelyhood 
of forgetting to check for an error and thus making the real error hard 
to find.

> fn2=`echo $fn1 | sed -e 's/odt/ps/' `
This invocation is the reason for this mail: You should (*) quote both 
fn1 and the result of the calculation. This would give:

fn2="`echo \"$fn1\" | sed -e 's/odt/ps/' `"

As you see, it's a bit awkward. That's why I recommend using $(...) 
instead of `...`:

fn2="$(echo "$fn1" | sed -e 's/odt/ps/')"

The given sed invocation will replace the _first_ occurence of "odt" 
(e.g. "Godtfred Kirk Christiansen.odt" -> "Gpsfred Kirk 
Christiansen.odt")  , BTW. You should append "$" after odt to make it 
match just the end of the string.

> if cat "$fn2" | grep -q "%!PS-Adobe-3.0"
Useless use of cat: you can use shell redirection instead:

if grep -q "%!PS-Adobe-3.0" < "$fn2"

> break;
Hmm, I don't see any loop that could be aborted. Do you mean "exit 0" 
instead?


(*) For this particular occurence, it isn't strictly necessary to fn1, 
as it is passed to echo which will behave the same either way. This 
isn't true for most other commands, so it's useful to develop a habit of 
always quoting arguments if they may contain arbitrary / unknown / 
"user"-specified data.

CU Sascha

-- 
http://sascha.silbe.org/
http://www.infra-silbe.de/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 489 bytes
Desc: Digital signature
Url : http://lists.sugarlabs.org/archive/sugar-devel/attachments/20090513/1c6113bb/attachment.pgp