Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: getopts #58

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix: getopts #58

wants to merge 1 commit into from

Conversation

mickenordin
Copy link
Member

Från [email protected]:

Hej!
Hittade ett par buggar i SUNET/multiverse/addhost och det verkar inte gå att skapa en issue, så jag mailar er istället. Den ena är ett resultat av commit “7323626” där Fredrik Thulin bytte ut getopt mot getopts getopts gillar inte shiftoperationer i while-loopen och genom att använda det i hanteringen av -n så äts nästa argument upp av shiftoperationen. Nästa bugg kom i commit “71e112e” av Micke Nordin. Den kopierar dels samma fel med shift, men har också tappat ett “:” i argumentlistan till getopts vilket gör att -n inte längre tar ett argument Här är några exempel på fel (Jag har skjutit in följande kod på rad 44 för att debugga) echo "cmd_hostname = $cmd_hostname"
echo "cmd_do_bootstrap = $cmd_do_bootstrap"
echo "cmd_fqdn = $cmd_fqdn"
echo "cmd_proxy = $cmd_proxy"
echo "proxyjump = $proxyjump"
exit 0
Exempel 1: fungerar för att det är “--” som äts upp av shift-operationen ./addhost -p proxyjump.example.com -- host1.example.com cmd_hostname = host1.example.com
cmd_do_bootstrap = no
cmd_fqdn = host1.example.com
cmd_proxy = proxyjump.example.com
proxyjump = -o ProxyJump=proxyjump.example.com
Exempel 2 fungerar inte för att det är host1.example.com som äts upp av shift-operationen ./addhost -p proxyjump.example.com host1.example.com Usage: ./addhost [-h] [-b] [-n fqdn] [--] []
-h show help
-b bootstrap (using ssh)
-n specify FQDN (if not the same as )
can be an IP number, or something that resolves to one
Exempel 3: fungerar inte som förväntat eftersom det saknas ett “:” efter “n” i getopts ./addhost -n host1.example.com 192.168.1.3
cmd_hostname = 192.168.1.3
cmd_do_bootstrap = no
cmd_fqdn = 192.168.1.3
cmd_proxy =
proxyjump =
Med min patch nedan får man istället följande resultat: Exempel 4: Har både -n och -p samt “--” för att markera att det är slut på options ./addhost -n host1.example.com -b -p proxyjump.example.com -- 192.168.1.3 cmd_hostname = 192.168.1.3
cmd_do_bootstrap = yes
cmd_fqdn = host1.example.com
cmd_proxy = proxyjump.example.com
proxyjump = -o ProxyJump=proxyjump.example.com
Exempel 5: Samma som exempel 4, men utan “--” (som är optional enligt Usage() ) ./addhost -n host1.example.com -b -p proxyjump.example.com 192.168.1.3 cmd_hostname = 192.168.1.3
cmd_do_bootstrap = yes
cmd_fqdn = host1.example.com
cmd_proxy = proxyjump.example.com
proxyjump = -o ProxyJump=proxyjump.example.com
Hälsningar,
Patrick Forsberg
Chalmers IRT
--- a/addhost 2024-10-18 20:30:09
+++ b/addhost 2024-10-18 20:30:44
@@ -13,12 +13,12 @@
echo " can be an IP number, or something that resolves to one"
}

-while getopts "bhnp:" this; do
+while getopts "bhn:p:" this; do
case "${this}" in
h) usage; exit 0;;
b) cmd_do_bootstrap="yes" ;;

  • n) cmd_fqdn="${OPTARG}" ; shift ;;
    
  • p) cmd_proxy="${OPTARG}" ; shift ;;
    
  • n) cmd_fqdn="${OPTARG}" ;;
    
  • p) cmd_proxy="${OPTARG}" ;;
    *) echo "Unknown option ${this}"; echo ""; usage; exit 1;;
    
    esac
    done

Hej!
Hittade ett par buggar i SUNET/multiverse/addhost och det verkar inte gå att skapa en issue, så jag mailar er istället.
Den ena är ett resultat av commit “7323626” där Fredrik Thulin bytte ut getopt mot getopts
getopts gillar inte shiftoperationer i while-loopen och genom att använda det i hanteringen av -n så äts nästa argument upp av shiftoperationen.
Nästa bugg kom i commit “71e112e” av Micke Nordin.
Den kopierar dels samma fel med shift, men har också tappat ett “:” i argumentlistan till getopts vilket gör att -n inte längre tar ett argument
Här är några exempel på fel (Jag har skjutit in följande kod på rad 44 för att debugga)
echo "cmd_hostname = $cmd_hostname"
echo "cmd_do_bootstrap = $cmd_do_bootstrap"
echo "cmd_fqdn = $cmd_fqdn"
echo "cmd_proxy = $cmd_proxy"
echo "proxyjump = $proxyjump"
exit 0
Exempel 1: fungerar för att det är “--” som äts upp av shift-operationen
./addhost -p proxyjump.example.com -- host1.example.com
cmd_hostname = host1.example.com
cmd_do_bootstrap = no
cmd_fqdn = host1.example.com
cmd_proxy = proxyjump.example.com
proxyjump = -o ProxyJump=proxyjump.example.com
Exempel 2 fungerar inte för att det är host1.example.com som äts upp av shift-operationen
./addhost -p proxyjump.example.com host1.example.com
Usage: ./addhost [-h] [-b] [-n fqdn] [--] [<host>]
 -h show help
 -b bootstrap <host> (using ssh)
 -n specify FQDN (if not the same as <host>)
 <host> can be an IP number, or something that resolves to one
Exempel 3: fungerar inte som förväntat eftersom det saknas ett “:” efter “n” i getopts
./addhost -n host1.example.com 192.168.1.3
cmd_hostname = 192.168.1.3
cmd_do_bootstrap = no
cmd_fqdn = 192.168.1.3
cmd_proxy =
proxyjump =
Med min patch nedan får man istället följande resultat:
Exempel 4: Har både -n och -p samt “--” för att markera att det är slut på options
./addhost -n host1.example.com -b -p proxyjump.example.com -- 192.168.1.3
cmd_hostname = 192.168.1.3
cmd_do_bootstrap = yes
cmd_fqdn = host1.example.com
cmd_proxy = proxyjump.example.com
proxyjump = -o ProxyJump=proxyjump.example.com
Exempel 5: Samma som exempel 4, men utan “--” (som är optional enligt Usage() )
./addhost -n host1.example.com -b -p proxyjump.example.com 192.168.1.3
cmd_hostname = 192.168.1.3
cmd_do_bootstrap = yes
cmd_fqdn = host1.example.com
cmd_proxy = proxyjump.example.com
proxyjump = -o ProxyJump=proxyjump.example.com
Hälsningar,
Patrick Forsberg
Chalmers IRT
--- a/addhost   2024-10-18 20:30:09
+++ b/addhost   2024-10-18 20:30:44
@@ -13,12 +13,12 @@
   echo " <host> can be an IP number, or something that resolves to one"
 }

-while getopts "bhnp:" this; do
+while getopts "bhn:p:" this; do
   case "${this}" in
      h) usage; exit 0;;
      b) cmd_do_bootstrap="yes" ;;
-     n) cmd_fqdn="${OPTARG}" ; shift ;;
-     p) cmd_proxy="${OPTARG}" ; shift ;;
+     n) cmd_fqdn="${OPTARG}" ;;
+     p) cmd_proxy="${OPTARG}" ;;
      *) echo "Unknown option ${this}"; echo ""; usage; exit 1;;
   esac
 done
Copy link

Copy link

@theseal theseal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTm

Copy link

@eest eest left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants