Re: [checks/scripts] [PATCH 4/4] bashisms update
On Sat, 2008-06-14 at 05:20 +0200, Frank Lichtenheld wrote:
> On Fri, Jun 13, 2008 at 03:09:18PM +0100, Adam D. Barratt wrote:
> > --- checks/scripts.orig 2008-06-13 14:51:32.000000000 +0100
> > +++ checks/scripts 2008-06-13 15:00:57.000000000 +0100
> > @@ -542,6 +542,11 @@
> > if ($cat_string eq "" and $checkbashisms and !$within_another_shell) {
> > my $found = 0;
> > my $match = '';
> > + my $LEADIN = '(?:(^|[`&;(|{])\s*|(if|do|while)\s+)';
> > + my @bashism_single_quote_regexs = (
> > + $LEADIN . 'echo\s+(?:-[^e]+\s+)?([\'])[^\']*(\\\[abcEfnrtv\\\0])+.*?[\']',
> > + # unsafe echo with backslashes
> > + );
>
> If someone has the time it might be interesting to see whether using
> qr'' instead of '' brings a measurable performance improvement.
My not hugely scientific testing (running checkbashisms trunk followed
by a copy using qr approximately 4000 times) suggests an average
improvement of around three seconds or six percent; not earth
shattering, but enough to justify the change.
For reference, the test file in each case contains 234 non-blank lines,
which checkbashisms flags 123 issues in.
As I suspected, the original patch didn't apply cleanly on top of the
three earlier patches; I've attached an updated patch against lintian
trunk which uses qr.
Regards,
Adam
--- checks/scripts.orig 2008-06-20 13:42:13.000000000 +0100
+++ checks/scripts 2008-06-20 18:20:30.000000000 +0100
@@ -543,41 +543,86 @@
if ($cat_string eq "" and $checkbashisms and !$within_another_shell) {
my $found = 0;
my $match = '';
+ my $LEADIN = qr'(?:(^|[`&;(|{])\s*|(if|do|while)\s+)';
+ my @bashism_single_quote_regexs = (
+ $LEADIN . qr'echo\s+(?:-[^e]+\s+)?([\'])[^\']*(\\[\\abcEfnrtv\\0])+.*?[\']',
+ # unsafe echo with backslashes
+ );
my @bashism_string_regexs = (
- '\$\[\w+\]', # arith not allowed
- '\$\{\w+\:\d+(?::\d+)?\}', # ${foo:3[:1]}
- '\$\{\w+(/.+?){1,2}\}', # ${parm/?/pat[/str]}
- '\$\{\#?\w+\[[0-9\*\@]+\]\}',# bash arrays, ${name[0|*|@]}
- '\$\{!\w+[\@*]\}', # ${!prefix[*|@]}
- '\$\{!\w+\}', # ${!name}
- '(\$\(|\`)\s*\<\s*\S+\s*(\)|\`)', # $(\< foo) should be $(cat foo)
- '\$RANDOM\b', # $RANDOM
- '\$(OS|MACH)TYPE\b', # $(OS|MACH)TYPE
- '\$HOST(TYPE|NAME)\b', # $HOST(TYPE|NAME)
- '\$DIRSTACK\b', # $DIRSTACK
- '\$EUID\b', # $EUID should be "id -u"
+ qr'\$\[\w+\]', # arith not allowed
+ qr'\$\{\w+\:\d+(?::\d+)?\}', # ${foo:3[:1]}
+ qr'\$\{\w+(/.+?){1,2}\}', # ${parm/?/pat[/str]}
+ qr'\$\{\#?\w+\[[0-9\*\@]+\]\}',# bash arrays, ${name[0|*|@]}
+ qr'\$\{!\w+[\@*]\}', # ${!prefix[*|@]}
+ qr'\$\{!\w+\}', # ${!name}
+ qr'(\$\(|\`)\s*\<\s*\S+\s*(\)|\`)', # $(\< foo) should be $(cat foo)
+ qr'\$\{?RANDOM\}?\b', # $RANDOM
+ qr'\$\{?(OS|MACH)TYPE\}?\b', # $(OS|MACH)TYPE
+ qr'\$\{?HOST(TYPE|NAME)\}?\b', # $HOST(TYPE|NAME)
+ qr'\$\{?DIRSTACK\}?\b', # $DIRSTACK
+ qr'\$\{?EUID\}?\b', # $EUID should be "id -u"
+ qr'\$\{?UID\}?\b', # $UID should be "id -ru"
+ qr'\$\{?SECONDS\}?\b', # $SECONDS
+ qr'\$\{?BASH_[A-Z]+\}?\b', # $BASH_SOMETHING
+ qr'\$\{?SHELLOPTS\}?\b', # $SHELLOPTS
+ qr'\$\{?PIPESTATUS\}?\b', # $PIPESTATUS
+ qr'\$\{?SHLVL\}?\b', # $SHLVL
+ qr'<<<', # <<< here string
+ $LEADIN . qr'echo\s+(?:-[^e]+\s+)?([\"])[^\"]*(\\[\\abcEfnrtv\\0])+.*?[\"]',
+ # unsafe echo with backslashes
);
my @bashism_regexs = (
- 'function \w+\(\s*\)', # function is useless
- # should be '.', not 'source'
- '(?:^|\s+)source\s+(?:\.\/|\/|\$)[^\s]+',
- '(\[|test|-o|-a)\s*[^\s]+\s+==\s', # should be 'b = a'
- '\s(\|\&)', # pipelining is not POSIX
- '[^\\\]\{([^\s]+?,)+[^\\\}\s]+\}', # brace expansion
- '(?:^|\s+)\w+\[\d+\]=', # bash arrays, H[0]
- '(?:^|\s+)read\s*(?:;|$)', # read without variable
- '(?:^|\s+)kill\s+-[^sl]\w*', # kill -[0-9] or -[A-Z]
- '(?:^|\s+)trap\s+["\']?.*["\']?\s+.*[1-9]', # trap with signal numbers
- '\&>', # cshism
- '(<\&|>\&)\s*((-|\d+)[^\s;|\)\`&]|[^-\d])', # should be >word 2>&1
- '\[\[(?!:)', # alternative test command
- '(?:^|\s+)select\s+\w+', # 'select' is not POSIX
- '(?:^|\s+)echo\s+-e', # echo -e
- '(?:^|\s+)exec\s+-[acl]', # exec -c/-l/-a name
- '(?:^|\s+)let\s', # let ...
- '(?<![\$\(])\(\(.*\)\)', # '((' should be '$(('
- '(\[|test)\s+-a', # test with unary -a (should be -e)
- '<<<', # <<< here string
+ qr'function \w+\(\s*\)', # function is useless
+ # should be '.', not 'source'
+ $LEADIN . qr'source\s+(?:\.\/|\/|\$)[^\s]+',
+ qr'(test|-o|-a)\s*[^\s]+\s+==\s', # should be 'b = a'
+ qr'\[\s+[^\]]+\s+==\s', # should be 'b = a'
+ qr'\s(\|\&)', # pipelining is not POSIX
+ qr'[^\\]\{(?:[^\s\\\}]+?,)+[^\\\}\s]+\}', # brace expansion
+ qr'(?:^|\s+)\w+\[\d+\]=', # bash arrays, H[0]
+ $LEADIN . qr'(read\s*(-[^r]+)*(?:;|$))',
+ # read without variable or with option other than -r
+ $LEADIN . qr'kill\s+-[^sl]\w*',# kill -[0-9] or -[A-Z]
+ $LEADIN . qr'trap\s+["\']?.*["\']?\s+.*[1-9]', # trap with signal numbers
+ qr'\&>', # cshism
+ qr'(<\&|>\&)\s*((-|\d+)[^\s;|)`&\\\\]|[^-\d\s]+)', # should be >word 2>&1
+ qr'\[\[(?!:)', # alternative test command
+ $LEADIN . qr'select\s+\w+', # 'select' is not POSIX
+ $LEADIN . qr'echo\s+(-n\s+)?-n?en?', # echo -e
+ $LEADIN . qr'exec\s+-[acl]', # exec -c/-l/-a name
+ qr'(?:^|\s+)let\s', # let ...
+ qr'(?<![\$\(])\(\(.*\)\)', # '((' should be '$(('
+ qr'\$\[[^][]+\]', # '$[' should be '$(('
+ qr'(\[|test)\s+-a', # test with unary -a (should be -e)
+ qr'/dev/(tcp|udp)', # /dev/(tcp|udp)
+ $LEADIN . qr'\w+\+=', # should be "VAR="${VAR}foo"
+ $LEADIN . qr'suspend\s',
+ $LEADIN . qr'caller\s',
+ $LEADIN . qr'complete\s',
+ $LEADIN . qr'compgen\s',
+ $LEADIN . qr'declare\s',
+ $LEADIN . qr'typeset\s',
+ $LEADIN . qr'disown\s',
+ $LEADIN . qr'builtin\s',
+ $LEADIN . qr'set\s+-[BHT]+', # set -[BHT]
+ $LEADIN . qr'alias\s+-p', # alias -p
+ $LEADIN . qr'unalias\s+-a', # unalias -a
+ $LEADIN . qr'local\s+-[a-zA-Z]+', # local -opt
+ $LEADIN . qr'local\s+\w+=', # local foo=bar
+ qr'(?:^|\s+)\s*\(?\w*[^\(\w\s]+\S*?\s*\(\)[^\"]?',
+ # function names should only contain [a-z0-9_]
+ $LEADIN . qr'(push|pop)d\b', # (push|pod)d
+ $LEADIN . qr'export\s+-[^p]', # export only takes -p as an option
+ $LEADIN . qr'ulimit\b',
+ $LEADIN . qr'shopt\b',
+ $LEADIN . qr'type\s',
+ $LEADIN . qr'time\s',
+ $LEADIN . qr'dirs\b',
+ qr'(?:^|\s+)[<>]\(.*?\)', # <() process substituion
+ qr'(?:^|\s+)readonly\s+-[af]', # readonly -[af]
+ $LEADIN . qr'(sh|\$\{?SHELL\}?) -[rD]', # sh -[rD]
+ $LEADIN . qr'(sh|\$\{?SHELL\}?) --\w+', # sh --long-option
+ $LEADIN . qr'(sh|\$\{?SHELL\}?) [-+]O', # sh [-+]O
);
# since this test is ugly, I have to do it by itself
@@ -593,9 +638,20 @@
}
}
+ my $line = $_;
+
+ unless ($found) {
+ for my $re (@bashism_single_quote_regexs) {
+ if ($line =~ m/($re)/) {
+ $found = 1;
+ ($match) = m/($re)/;
+ last;
+ }
+ }
+ }
+
# Ignore anything inside single quotes; it could be an
# argument to grep or the like.
- my $line = $_;
# $cat_line contains the version of the line we'll check
# for heredoc delimiters later. Initially, remove any
--- testset/maintainer-scripts/debian/prerm.orig 2008-06-20 13:42:44.000000000 +0100
+++ testset/maintainer-scripts/debian/prerm 2008-06-20 13:41:24.000000000 +0100
@@ -43,6 +43,7 @@
# More false positives for bashism checks. None of these are errors.
echo "$line" | grep -q '{fonts/map,}/{\$progname,pdftex,dvips,}//'
+echo "$line" | grep -q "${fonts},${foo}"
echo '$[1+2]'
printf "foo |& bar"
perl -e "print q( kill -HUP $? )"
@@ -55,3 +56,100 @@
# This is the only install-sgmlcatalog call that's allowed.
install-sgmlcatalog --quiet --remove package
+
+# More bashisms checks
+
+read -x foo
+read -x
+read -r foo
+read foo
+read
+
+echo "a\\b"
+echo 'a\nb'
+
+echo "${UID}"
+echo "$EUID"
+echo "$SHLVL"
+echo "$DIRSTACK"
+echo "$SECONDS"
+echo "$BASH"
+echo "$BASH_FOO"
+echo "$SHELLOPTS"
+echo "$PIPESTATUS"
+
+bar="$(cut '-d|' -f2 <<< "$foo")"
+
+VAR=1
+VAR+=a
+
+echos() {
+ echo -n -e "bar"
+ echo -e -n "bar"
+ echo -en "bar"
+ echo -ne "bar"
+ echo "bar"
+ echo "echo -e foo"
+}
+
+ech.os() {
+ echo foo >& 2
+ echo foo >&bar
+ echo foo >& bar
+}
+
+echoes() {
+ echo "abc\nxyz"
+ echo 'xyz\rabc'
+ echo foo\cbar
+
+ echo -e "abc\nxyz"
+ echo -net 'xyz\rabc'
+ echo -e foo\cbar
+}
+
+foobar.() {
+ suspend x
+ suspended x
+ caller x
+ complete x
+ compgen x
+ declare -a foo
+}
+
+.foobar() {
+ typeset -x bar
+ disown 1
+ builtin foo
+ set -B
+ alias -p
+ unalias -a
+}
+
+IFS="()"
+
+ulimit
+shopt
+type -v bar
+time ls
+dirs
+diff <(tac a) <(tac b)
+
+pushd
+
+local foo=bar
+local -x foo
+
+popd
+
+readonly -f
+
+echo bar > /dev/tcp
+export x
+export -x x
+export -p x
+
+sh -x
+sh -D
+sh --foo
+sh +O
--- testset/tags.maintainer-scripts.orig 2008-06-20 13:42:27.000000000 +0100
+++ testset/tags.maintainer-scripts 2008-06-20 13:40:57.000000000 +0100
@@ -9,7 +9,7 @@
E: maintainer-scripts: install-sgmlcatalog-deprecated postinst:98
E: maintainer-scripts: install-sgmlcatalog-deprecated postrm:46
E: maintainer-scripts: interpreter-without-predep control/config #!/usr/bin/python
-E: maintainer-scripts: maintainer-script-calls-init-script-directly prerm:54
+E: maintainer-scripts: maintainer-script-calls-init-script-directly prerm:55
E: maintainer-scripts: maintainer-script-does-not-check-for-existence-of-wm-menu-config postinst:31
E: maintainer-scripts: maintainer-script-modifies-inetd-conf postinst:91
E: maintainer-scripts: maintainer-script-modifies-inetd-conf postinst:92
@@ -64,6 +64,40 @@
W: maintainer-scripts: possible-bashism-in-maintainer-script postinst:20 'H[0]='
W: maintainer-scripts: possible-bashism-in-maintainer-script postinst:21 '${H[0]}'
W: maintainer-scripts: possible-bashism-in-maintainer-script postinst:23 '${H[@]}'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:102 ' echo "abc\nxyz"'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:103 ' echo 'xyz\rabc''
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:106 ' echo -e'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:107 ' echo -ne'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:108 ' echo -e'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:111 'foobar.() '
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:112 ' suspend '
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:114 ' caller '
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:115 ' complete '
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:116 ' compgen '
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:117 ' declare '
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:120 '.foobar() '
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:121 ' typeset '
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:122 ' disown '
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:123 ' builtin '
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:124 ' set -B'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:125 ' alias -p'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:126 ' unalias -a'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:131 'ulimit'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:132 'shopt'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:133 'type '
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:134 'time '
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:135 'dirs'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:136 ' <(tac a)'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:138 'pushd'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:140 'local foo='
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:141 'local -x'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:143 'popd'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:145 'readonly -f'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:147 '/dev/tcp'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:149 'export -x'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:153 'sh -D'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:154 'sh --foo'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:155 'sh +O'
W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:19 '[ "$2" == '
W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:23 'function foo( )'
W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:26 '&>'
@@ -71,7 +105,29 @@
W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:31 'trap "echo hi" EXIT HUP 3'
W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:33 '[['
W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:34 ' kill -HUP'
-W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:51 '${line:3:1}'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:52 '${line:3:1}'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:62 'read -x foo'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:63 'read -x'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:66 'read'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:68 'echo "a\\b"'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:69 'echo 'a\nb''
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:71 '${UID'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:72 '$EUID'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:73 '$SHLVL'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:74 '$DIRSTACK'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:75 '$SECONDS'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:77 '$BASH_FOO'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:78 '$SHELLOPTS'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:79 '$PIPESTATUS'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:81 '<<<'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:84 'VAR+='
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:87 ' echo -n -e'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:88 ' echo -e'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:89 ' echo -en'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:90 ' echo -ne'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:95 'ech.os() '
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:97 '>&bar'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:98 '>& bar'
W: maintainer-scripts: possibly-insecure-handling-of-tmp-files-in-maintainer-script postinst:50
W: maintainer-scripts: possibly-insecure-handling-of-tmp-files-in-maintainer-script postrm:39
W: maintainer-scripts: postinst-does-not-load-confmodule
@@ -79,5 +135,10 @@
W: maintainer-scripts: postrm-does-not-purge-debconf
W: maintainer-scripts: postrm-has-useless-call-to-ldconfig
W: maintainer-scripts: read-in-maintainer-script postinst:18
+W: maintainer-scripts: read-in-maintainer-script prerm:62
+W: maintainer-scripts: read-in-maintainer-script prerm:63
+W: maintainer-scripts: read-in-maintainer-script prerm:64
+W: maintainer-scripts: read-in-maintainer-script prerm:65
+W: maintainer-scripts: read-in-maintainer-script prerm:66
W: maintainer-scripts: start-stop-daemon-in-maintainer-script postinst:159
W: maintainer-scripts: update-alternatives-remove-called-in-postrm
Reply to: