0

I have a Bash script which tests three parameters before continuing, else exits and prints a help message. It currently works, but the test seems lengthy and difficult to read.

The following (ignoring case) must be true before the script continues:

  • Three parameters are used
  • $1 is either home5, home2 or guest
  • $2 matches one line found in a text file containing country codes (UK, DE, FR etc.)
  • $3 is either udp or tcp

Sample from 'countries.txt'

BD
BE
BF
BG
BA

Script

#!/bin/bash

ID=$1

print_help () {
  printf "\nParameters missing or invalid.\n\nUsage:\n  connect [Wi-Fi connection ID] [VPN country] [VPN protocol]\n\nExample:\n  connect home5 uk tcp\n\n"
}

check_status () {
  if [[ $? -ne 0 ]]
  then
    exit 1
  fi
}

if [[ ( -z $1 ) || ( -z $2 ) || ( -z $3 ) || ( $1 -ne "home5" ) || ( $1 -ne "home2" ) || ( $1 -ne "guest" ) || ( $(egrep -i $2 Scripts/countries.txt) -ne 0 ) || ( $3 -ne "udp" ) || ( $3 -ne "tcp" ) ]]
then
  print_help
  exit 1
fi

if [[ $ID == "home5" ]]
then
  ssid="Home Network (5 GHz)"
elif [[ $ID == "home2" ]]
then
  ssid="Home Network (2.4 GHz)"
elif [[ $ID == "guest" ]]
then
  ssid="Guest Network (2.4 GHz)"
else
  print_help
  exit 1
fi 2> /dev/null
printf "\nConnecting to $ssid...\n"
nmcli con up "$ID" 1> /dev/null
check_status
sleep 1

printf "\nConnecting to VPN...\n"
vpn c --cc $2 -p $3
check_status
echo
exit 0
  • The second criteria is unclear. Is it ok to find $2 in the file if $2 is just U? Is $2 to be treated as a regular expression, so that .. matches any two-letter country code? Is the list of country codes in the text files on a single line or on separate lines? You are not quoting any of your variable expansions, does that mean that you want to allow wildcards that may expand to filenames (in e.g. $1)? – Kusalananda Oct 14 at 6:44
  • @Kusalananda, Thanks for comment. I've updated post to the best of my ability. I'm not too sure what you mean by the following: "Is $2 to be treated as a regular expression, so that .. matches any two-letter country code?", "You are not quoting any of your variable expansions, does that mean that you want to allow wildcards that may expand to filenames (in e.g. $1)?" - I'm not familiar with the difference between quoting and not quoting variable expansions. – squire Oct 14 at 7:15
  • if [[ $1:$3 =~ ^(home[52]|guest):(tcp|udp)$ ]] && fgrep -wqe "$2" country_codes; then proceed ...; else error out ...; fi – user414777 Oct 14 at 7:25
  • I've tested the above but it doesn't seem to work; the test fails with correct parameters. – squire Oct 14 at 7:31
  • 3
    Note that the grep man page says: "Direct invocation as either egrep or fgrep is deprecated" -- use grep -Fiqw – glenn jackman Oct 14 at 12:48
0
COUNTRIES=Scripts/countries.txt

if [[ $1:$3 =~ ^(home[52]|guest):(tcp|udp)$ ]] && grep -Fiqwe "$2" "$COUNTRIES"
then
  (proceed)
else
  (print help and exit)
fi
| improve this answer | |
  • 2
    I am curious: Why did you quote $2 but not $COUNTRIES? Also, would you mind editing your post to add some explanation on how this works? Now everyone may be familiar with the "matches regex"-Operater =~ and its application. – AdminBee Oct 14 at 8:36
  • 1
    Is there a reason you did not turn this into one if statement per criteria, which woud have made it easier to understand and to maintain? – Kusalananda Oct 14 at 8:52
  • Think fgrep is deprecated, grep -F is preferred. – bac0n Oct 14 at 13:18
  • @AdminBee, I didn't know there was much difference between quoting/not quoting (would appreciate an explanation). I simply used "$2" as per @user414777's earlier suggestion, then when testing forgot to quote the new $COUNTRIES variable. Regarding editing the post, do you mean the main post or the answer, and explanation on how =~ works or how the whole script is meant to work? – squire Oct 15 at 5:54
  • @Kusalananda, I tried to reduce lines/words as much as possible, which I thought was better. Readability isn't too much of an issue for my purposes as this script will only be used by myself. But I still wanted to know the best/preferred syntax. – squire Oct 15 at 5:58

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.