eczema / ciscocmd-cosi

ciscocmd repository for COSI
https://sourceforge.net/projects/cosi-nms/files/ciscocmd/
GNU General Public License v2.0
13 stars 8 forks source link

Ciscocmd fails with bash 4.4, but not with 4.3. #2

Open alvarezp opened 7 years ago

alvarezp commented 7 years ago

A Debian Stretch user named Denis (alias "sharlino") is reporting problems ciscocmd under Bash 4.4 but not 4.3. I quote his message:

ciscocmd is not working as expected with bash 4.4 (and maybe higher). On today, 26-12-2016, the recent version of bash in the Debian Stretch is 4.4.2. Downgrading bash from 4.4 to 4.3 has solved the problem. I think the problem is in the IFS, particularly in the delimeters, because of:

The `mapfile' builtin now has a -d option to use an arbitrary character as the record delimiter, and a -t option to strip the delimiter as supplied with -d.

as per as https://lists.gnu.org/archive/html/info-gnu/2016-09/msg00008.html

I'm raising this issue for the proper follow up. I'll point the user to this issue.

alvarezp commented 7 years ago

To troubleshoot I patched ciscocmd on a system with bash 4.4 and another one with 4.3 like this:

$ diff -u $(which ciscocmd) ciscocmd
--- /home/alvarezp/bin/ciscocmd 2013-09-02 11:54:52.435133325 -0700
+++ ciscocmd    2017-01-02 14:44:33.772367925 -0800
@@ -59,9 +59,13 @@
 log_user 0
 # first: take arguments from environment variable

+send_user -- "[lrange [array get env ARGS] 1 end]\n"
 regsub -all -- "{|}" [lrange [array get env ARGS] 1 end] "" argv
+send_user -- "$argv\n"
 regsub -all -- "²" $argv "\" \"" argv
+send_user -- "$argv\n"
 regsub -all -- "^|$" $argv "\"" argv
+send_user -- "$argv\n"

 if { $argv == "" } {
    set argv "-h"

... and there are differences with the argument parsing. On Bash 4.3:

$ ./ciscocmd -t 127.0.0.1 -Y -c 'show run' -u alvarezp
{-t²127.0.0.1²-Y²-c²show run²-u²alvarezp}
-t²127.0.0.1²-Y²-c²show run²-u²alvarezp
-t" "127.0.0.1" "-Y" "-c" "show run" "-u" "alvarezp
"-t" "127.0.0.1" "-Y" "-c" "show run" "-u" "alvarezp"
Processing... 127.0.0.1^C

On Bash 4.4:

$ ./ciscocmd -t 127.0.0.1 -Y -c 'show run' -u alvarezp
{-t 127.0.0.1 -Y -c show run -u alvarezp}
-t 127.0.0.1 -Y -c show run -u alvarezp
-t 127.0.0.1 -Y -c show run -u alvarezp
"-t 127.0.0.1 -Y -c show run -u alvarezp"
Please, enter hostlist: ^C^C
alvarezp commented 7 years ago

So...

bash -c 'IFS=":"; export A=$@; env | grep ^A=' -- a b c

On Bash 4.3:

A=a:b:c

On Bash 4.4:

A=a b c
alvarezp commented 7 years ago

Turns out that that the usage of $@ on line 78 is incorrect. The proper way to do this is by using "$*" (with double quotes) on line 78. The fact that it worked on Bash 4.3 is because of a bug in Bash. This was fixed on Bash 4.4 in this commit:

I fixed this for ciscocmd on my repo in https://github.com/alvarezp/ciscocmd-cosi/commit/dcf8956987eea163f37374acdf0f9cd771ebe05a.

Hopefully this doesn't break anything else.

So many thanks goes to Eduardo Bustamante for the detailed explanation, helping me save a lot of research time and finding the proper fix.

aaronhurt commented 7 years ago

I have another solution in #5 that just removes the bash dependency entirely.

tpo commented 6 years ago

@alvarezp , since there haven't been patches applied to @eczema 's git repo, I'd suggest you merge useful patches from other trees - such as @leprechau 's cleanup patches (not the bash removal patch, if that one's controversial) - into your tree?