On 19Mar2009 18:55, Daniel B. Thurman <dant@xxxxxxxxx> wrote: > Cameron Simpson wrote: >> Ok, but I strongly recommend you never us a regexp unquoted - the >> necessary backslash nesting gets nasty real fast. A better way is like >> this: >> re='s/b/h/' >> sed -e "$re" [...] >> The point here, unrelated to the pipe exit status issue, is to keep the >> sed stuff easy to write and undamaged by shell substitution. [...] > > Another person sent me private email > warning of the double-quotes, but thanks > for your information, it is interesting! > > This is what I ended up doing, so please give > your constructive critiques, if you so like? Untested, but here is how I would write each of you code pieces were I doing it myself: > TRACKER="Tracker.log" > SFILE="Bad_Hosts_File.txt" > TFILE="Bad_Hosts_File_$$.txt" tracker=Tracker.log sfile=Bad_Hosts_File.txt tfile=Bad_Hosts_File_$$.txt Your quotes are unneeded here. Plenty of people find the "" marks and in similar cases, to use ${foo} instead of $foo elsewhere, but I find the syntactic noise annoying if there's no other necessity. Also, you should never use UPPER CASE names for script local variables (i.e. that are not to be exported). I give some background why here: http://markmail.org/message/awimhb3nmu6ssvwp > pat="EACC" pat=EACC > re1="-e '/$pat/s/^.*Received:\[from\s\+\(.*\)\s\+(.*/\1/'" > re2="-e '/^.*cdkkt.com$/d'" Note that this "$" should probably be backslashed. As it happens, $/ is not a shell special variable, so it is left alone. However, has you used $# or one of the others ($!, et al) there would have been substitution done, mangling your sed code. > rex="sed $re1 $re2" I would strongly reommend putting this in a sed script file, perhaps like this: cat >sedf$$ <<X /$pat/s/^.*Received:\[from\s\+\(.*\)\s\+(.*/\1/ /^.*cdkkt.com\$/d X rex="sed -f sedf$$" Hmm, actually, maybe not. Maybe like this: sedline="/$pat/s/^.*Received:\[from\s\+\(.*\)\s\+(.*/\1/; /^.*cdkkt.com\$/d" and then: sed -e "$sedline" in your pipeline lower down. > echo -en "\033[0;36m>> \033[0;35mExtracting data from:\n \ > [\033[0;34m${TRACKER}\033[0;35m and appending to\n \ > [\033[0;34m${TFILE}\033[0;35m]\033[0m" I usually put escape sequences into their own variables for readbility, like this: http://www.cskk.ezoshosting.com/cs/rc/shell/colour_ansi Then you can talk about ${tty_green}, ${tty_normal} etc in your text; somewhat more readable. You can do even better than this, actually. Since not all terminals use the ANSI colour escapes, it is better to use tput, like this: http://www.cskk.ezoshosting.com/cs/css/bin/with-colour and then in your script: esc_tracker=`with-colour green echo "$TRACKER"` and then use ${esc_tracker} in messages. And so forth. > out=$(grep "$pat" "${TRACKER}" | \ > eval "$rex" | sort -n | \ > uniq >> "${TFILE}"); ret="$?"; > > if [ "$ret" -gt 0 ] || \ > [ "$PIPESTATUS[0]" -gt 0 ] || \ > [ "$PIPESTATUS[1]" -gt 0 ] || \ > [ "$PIPESTATUS[2]" -gt 0 ] || \ > [ "$PIPESTATUS[3]" -gt 0 ]; then > echo -e " \033[0;31mFailed.\n \ > \033[0;35m-- ErrorStatus:<ret=$ret>,\ > PipeStatus:<$PIPESTATUS[@]>\n\ > -- out:<$out>\033[0m" > echo -e "\033[0;31m>> \033[0;31m$PROG exited.\033[0m\n" > exit 1 > fi > echo -e " \033[0;32mDone.\033[0m"; > > I guess the ugly part is the multiple $PIPESTATUS[n] > in the if statement test block. but perhaps this is the best > I can do? Nah. You can do this: if out=$(grep "$pat" "$tracker" | $rex | sort -un >>"$tfile") then case " $PIPESTATUS" in *' '[1-9]*) echo exit ok, but PIPESTATUS=$PIPESTATUS ;; *)echo exit ok and PIPESTATUS all zeroes ;; esac else echo exit not ok fi Um, you are aware that $out will always be empty? You have sent standard out to the temp file. Standard error is not collected by back ticks or $(); it will be displayed directly and not land in $out. You probably need to go: out=$(exec 2>&1; grep .... >>"$tfile") An easy test is like this: out=$(ls /no/such/file >>temp) echo "out=[$out]" versus: out=$(exec 2>&1; ls /no/such/file >>temp) echo "out=[$out]" Cheers, -- Cameron Simpson <cs@xxxxxxxxxx> DoD#743 http://www.cskk.ezoshosting.com/cs/ The aim of AI is to make computers act like the ones in the movies. - Graham Mann -- fedora-list mailing list fedora-list@xxxxxxxxxx To unsubscribe: https://www.redhat.com/mailman/listinfo/fedora-list Guidelines: http://fedoraproject.org/wiki/Communicate/MailingListGuidelines