On Wed, 2009-05-27 at 19:11 -0500, Raphael Geissert wrote:
Thanks for the list. I've included comments on each issue inline.
Where issues don't have an immediate fix I've cloned them to new bugs to
make tracking stuff easier.
> -----------------
> FP:
> > possible bashism in
> > ./usr/share/pyshared/support-files/setuptools-0.6c9-py2.4.egg line 202
>
> Fix (allow exec to be preceded by 'then'):
> @@ -408,7 +409,7 @@ sub script_is_evil_and_wrong {
> last if (++$i > 55);
> if (m~
> # the exec should either be "eval"ed or a new statement
> - (^\s*|\beval\s*[\'\"]|(;|&&)\s*)
> + (^\s*|\beval\s*[\'\"]|(;|&&|\bthen)\s*)
Yep, no problem with that.
> -----------------
> FP:
> > possible bashism in ./usr/share/shorewall6-lite/lib.base line 684 (sourced
> > script with arguments):
> > . $(find_file $(expand $@))
>
> Workaround (this needs to be fixed by stripping evals, $(), ``, and any other
> form of code execution and looking for bashisms in those parts individually):
> Apply the same dummy logic used for "" and '' to $()
>
> @@ -281,8 +282,8 @@ foreach my $filename (@ARGV) {
> # detect source (.) trying to pass args to the command it runs
> # The first expression weeds out '. "foo bar"'
> if (not $found and
> - not m/^\s*\.\s+(\"[^\"]+\"|\'[^\']+\')\s*(\&|\||\d?>|<|;|\Z)/
> - and m/^\s*(\.\s+[^\s;\`:]+\s+([^\s;]+))/) {
> + not m/^\s*\.\s+(\"[^\"]+\"|\'[^\']+\'|\$\([^)]+\)+)\s*(\&|\||
> \d?>|<|;|\Z)/
> + and m/\s*(\.\s+[^\s;\`:]+\s+([^\s;]+))/) {
Hmmm, this seems a little hacky, but I suppose it's better than nothing
in the short term. :-/ As you noted yourself later on, it doesn't cope
well with bracketed groups which occur inside $().
Cloned as #530905 so we can try and find a better fix.
> -----------------
> FN:
>
> > if something; then . foo bar; else bar; fi
>
> Fix (+ move LEADIN to the global scope):
> @@ -281,8 +282,8 @@ foreach my $filename (@ARGV) {
> # detect source (.) trying to pass args to the command it runs
> # The first expression weeds out '. "foo bar"'
> if (not $found and
> - not m/^\s*\.\s+(\"[^\"]+\"|\'[^\']+\')\s*(\&|\||\d?>|<|;|\Z)/
> - and m/^\s*(\.\s+[^\s;\`:]+\s+([^\s;]+))/) {
> + not m/$LEADIN\.\s+(\"[^\"]+\"|\'[^\']+\'|\$\([^)]+\)+)\s*(\&|
> \||\d?>|<|;|\Z)/
> + and m/$LEADIN(\.\s+[^\s;\`:]+\s+([^\s;]+))/) {
Yep, okay. Why "our $LEADIN" though? It's global scope, so why not just
"my"?
> -----------------
>
> FP (new kind of wrapper):
>
> usr/share/doc/systemtap-doc/examples/process/errsnoop.stp:
> > #!/bin/sh
> > //usr/bin/env stap -DMAXMAPENTRIES=20480 $0 $@; exit $?
> > # errsnoop.stp
> > ...
Cloned as #530906
> -----------------
>
> FP (ref: #530084):
> > if false; then foo; else exec something; fi
>
> @@ -408,7 +409,7 @@ sub script_is_evil_and_wrong {
> last if (++$i > 55);
> if (m~
> # the exec should either be "eval"ed or a new statement
> - (^\s*|\beval\s*[\'\"]|(;|&&)\s*)
> + (^\s*|\beval\s*[\'\"]|(;|&&|\b(then|else))\s*)
Couldn't you have included that with the "then" fix at the top? ;-)
> FN:
>
> > #!/bin/sh
> > cat <<FOO
> > hello
> > $(echo -e "world\c")
> >
> > Running on $OSTYPE
> > FOO
>
> Only here docs with quoted markers should be ignored.
Well... no. And yes. Some tests need to be skipped in this case, yes,
but by no means all. Remove the $() around the echo, for instance, and
it becomes literal text which shouldn't be flagged.
This is now #530907.
Regards,
Adam
--
To UNSUBSCRIBE, email to [email protected]
with a subject of "unsubscribe". Trouble? Contact [email protected]
