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/

Attachment: signature.asc
Description: Digital signature

_______________________________________________
Sugar-devel mailing list
Sugar-devel@lists.sugarlabs.org
http://lists.sugarlabs.org/listinfo/sugar-devel

Reply via email to