Re: checks/scripts / checkbashsims updates
Adam D. Barratt [EMAIL PROTECTED] writes: Sorry to be a pain, but we've tightened this further in svn to '(?![\$\(])\(\(.*\)\)' so as not to match constructs such as progress_size=$(((100 - $PROGRESS_STATE) / 3)) What *is* this construct trying to catch, anyway? dash supports things like $((4 + 5)), and if the above is also okay, I'm not sure when this would not be a false positive. -- Russ Allbery ([EMAIL PROTECTED]) http://www.eyrie.org/~eagle/ -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
Re: checks/scripts / checkbashsims updates
On Tue, 2008-02-19 at 09:07 -0800, Russ Allbery wrote: Adam D. Barratt [EMAIL PROTECTED] writes: Sorry to be a pain, but we've tightened this further in svn to '(?![\$\(])\(\(.*\)\)' so as not to match constructs such as progress_size=$(((100 - $PROGRESS_STATE) / 3)) What *is* this construct trying to catch, anyway? dash supports things like $((4 + 5)), and if the above is also okay, I'm not sure when this would not be a false positive. So far as I can tell, it's intended to be catching use of (( as a synonym for let, instead of $((, for performing arithmetic expansion. (The accompanying comment simply says (( should be $((.) The check was added approximately 18 months ago but unfortunately there's no clue in the check in comment, which refers to other changes made in the same commit. Julian? Adam -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
Re: checks/scripts / checkbashsims updates
Adam D. Barratt [EMAIL PROTECTED] writes:
Please let me know how you'd prefer to handle these, and any such issues
that arise in future. I'm more than happy to simply provide a list (as
below), send patches to the list or file bugs against lintian with
patches.
Either patches to the list or bugs against lintian with a list would be
ideal for me.
The first issue is from the checkbashisms side:
'(?!\$)\(\(', # '((' should be '$(('
This will also match constructs such as if ((foo || bar) baz). I've
tightened the expression in checkbashisms SVN to be '(?!\$)\(\(.*\)\)'.
Done likewise in Subversion for lintian.
The second issue that was highlighted is the check for arguments being
passed to scripts that are sourced (i.e. . foo bar). There are a
number of constructs that the current check flags, which so far as I can
tell are all legitimate:
. ${sysconfdir}/pop3d; \
. ${sysconfdir}/pop3d-ssl ; \
. `dirname $0`/guilt
I've modified the check to:
if (not $found and m/^\s*(\.\s+[^\s;]+\s+([^\s]+))/) {
which allows all of the above. The only problem I've found with it is
that constructs such as
. `foo bar` `foobar baz`
aren't flagged, but IMHO that's the lesser of two evils.
I assume that you mean [^\s;\`] here instead of only ;. Done similarly in
lintian.
Finally, script_is_evil_and_wrong() allows ;exec foo but not ; exec
foo. I'm assuming this was a simple oversight, as the two forms are
equivalent.
Added \s* after ; in the regex. Thanks!
--
Russ Allbery ([EMAIL PROTECTED]) http://www.eyrie.org/~eagle/
--
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
Re: checks/scripts / checkbashsims updates
On Mon, 2008-02-18 at 16:33 -0800, Russ Allbery wrote:
Adam D. Barratt [EMAIL PROTECTED] writes:
Please let me know how you'd prefer to handle these, and any such issues
that arise in future. I'm more than happy to simply provide a list (as
below), send patches to the list or file bugs against lintian with
patches.
Either patches to the list or bugs against lintian with a list would be
ideal for me.
Okay, thanks. I'll bear that in mind.
[...]
The second issue that was highlighted is the check for arguments being
passed to scripts that are sourced (i.e. . foo bar). There are a
number of constructs that the current check flags, which so far as I can
tell are all legitimate:
. ${sysconfdir}/pop3d; \
. ${sysconfdir}/pop3d-ssl ; \
. `dirname $0`/guilt
I've modified the check to:
if (not $found and m/^\s*(\.\s+[^\s;]+\s+([^\s]+))/) {
which allows all of the above. The only problem I've found with it is
that constructs such as
. `foo bar` `foobar baz`
aren't flagged, but IMHO that's the lesser of two evils.
I assume that you mean [^\s;\`] here instead of only ;. Done similarly in
lintian.
I didn't, but it works better, so I do now. :-) The final [^\s]+ group
alwo wants to be [^\s;]+ or
. ${sysconfdir}/pop3d-ssl ; \
is still flagged. (Adding ` to that group didn't seem to make any
difference to my test expressions).
Adam
--
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
Re: checks/scripts / checkbashsims updates
Adam D. Barratt [EMAIL PROTECTED] writes:
I didn't, but it works better, so I do now. :-) The final [^\s]+ group
alwo wants to be [^\s;]+ or
. ${sysconfdir}/pop3d-ssl ; \
is still flagged. (Adding ` to that group didn't seem to make any
difference to my test expressions).
Ah, yes, thanks. That change has now also been made in lintian.
--
Russ Allbery ([EMAIL PROTECTED]) http://www.eyrie.org/~eagle/
--
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
Re: checks/scripts / checkbashsims updates
On Mon, 2008-02-18 at 16:33 -0800, Russ Allbery wrote:
Adam D. Barratt [EMAIL PROTECTED] writes:
[...]
The first issue is from the checkbashisms side:
'(?!\$)\(\(', # '((' should be '$(('
This will also match constructs such as if ((foo || bar) baz). I've
tightened the expression in checkbashisms SVN to be '(?!\$)\(\(.*\)\)'.
Done likewise in Subversion for lintian.
Sorry to be a pain, but we've tightened this further in svn to
'(?![\$\(])\(\(.*\)\)'
so as not to match constructs such as
progress_size=$(((100 - $PROGRESS_STATE) / 3))
Adam
--
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
checks/scripts / checkbashsims updates
Hi,
Thanks for adding the set of bashism checks from checkbashisms to
lintian recently. As a result of the recent bugs that people have been
filing on other packages using checkbashsims output, a few issues have
surfaced - one with a check that originated in the patch, and a couple
that were already in lintian, just for balance. :-)
Please let me know how you'd prefer to handle these, and any such issues
that arise in future. I'm more than happy to simply provide a list (as
below), send patches to the list or file bugs against lintian with
patches.
The first issue is from the checkbashisms side:
'(?!\$)\(\(', # '((' should be '$(('
This will also match constructs such as if ((foo || bar) baz). I've
tightened the expression in checkbashisms SVN to be '(?!\$)\(\(.*\)\)'.
The second issue that was highlighted is the check for arguments being
passed to scripts that are sourced (i.e. . foo bar). There are a
number of constructs that the current check flags, which so far as I can
tell are all legitimate:
. ${sysconfdir}/pop3d; \
. ${sysconfdir}/pop3d-ssl ; \
. `dirname $0`/guilt
I've modified the check to:
if (not $found and m/^\s*(\.\s+[^\s;]+\s+([^\s]+))/) {
which allows all of the above. The only problem I've found with it is
that constructs such as
. `foo bar` `foobar baz`
aren't flagged, but IMHO that's the lesser of two evils.
Finally, script_is_evil_and_wrong() allows ;exec foo but not ; exec
foo. I'm assuming this was a simple oversight, as the two forms are
equivalent.
Adam
--
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
