FW: [Gllug] Awk Sed

[tet at accucard.com]

>some quick suggestions (if you must use sed :)

Agreed, but:

>for file in $*; do

This will fail with filenames containing whitespace. Use instead:

	for file in "$@"; do

Quote $file everywhere it's dereferenced for the same reasons.

>  sed -e "s/$pat1/$pat2/g" $file > ".harrytemp$file" && mv ".harrytemp$file" $file

If "$file" is specified as a full path, then your temporary filename
will be invalid, and the redirect will fail. Equally, if "$file" is
on a separate filesystem to the current directory, then the move won't
be atomic, and hence may be prone to file corruption if it fails part
way through. If you instead base the temporary filename on the file
you're altering, it'll work fine. Plus it's always good practice to
make temporary filenames pseudo-unique, by using the PID:

	sed "s/$pat1/$pat2/g" "$file" > "$file.$$" && mv "$file.$$" "$file"

Also, commenting on Harry's original script:

Since it's a shell script, you don't need to put everything on a
single semicolon-separated line.

As has been mentioned in the past couple of days, /bin/sh isn't always
POSIX compliant, so if you want your script to be portable, either use
ksh, or avoid the $(ls $3) construct. You could use backquotes to
achieve the same thing in a portable manner. The only downside to them
is that they don't nest. But you don't need that here anyway.

Using $(ls $3) won't do what you expect. Since the shell expands *.txt
before it passes the arguments to harrysed, $3 will only pick up the
first .txt file in the current directory. You can use strip of and save
the first two arguments with shift, and then use "$@" to get at all
the others.

Hope that helps.

Tet

-- 
Gllug mailing list  -  Gllug at linux.co.uk
http://list.ftech.net/mailman/listinfo/gllug