Lots of little tweaks that should never hurt. Some of them may help in

corner cases; others probably make no difference.

Signed-off-by: Jim Pryor <profjim@jimpryor.net>

8 files changed, 56 insertions, 48 deletions

diff --git a/src/connections/ethernet b/src/connections/ethernet
index 22405ad..5985ce5 100644
--- a/src/connections/ethernet
+++ b/src/connections/ethernet
@@ -4,8 +4,8 @@
 ethernet_up() {
     load_profile "$1"
 
-    if [[ ! -e /sys/class/net/"$INTERFACE" ]]; then
-        if ! echo "$INTERFACE"| fgrep ":"; then
+    if [[ ! -e "/sys/class/net/$INTERFACE" ]]; then
+        if ! echo "$INTERFACE" | fgrep -q ":"; then
             report_fail "interface $INTERFACE does not exist"
             return 1
         fi
@@ -21,7 +21,7 @@ ethernet_up() {
     fi
 
     if checkyesno "${AUTH8021X:-no}"; then 
-        . "${SUBR_DIR}"/8021x
+        . "$SUBR_DIR/8021x"
         [[ -z "$WPA_CONF" ]] && WPA_CONF="/etc/wpa_supplicant.conf"
         [[ -z "$WPA_OPTS" ]] && WPA_OPTS="-Dwired"
         report_debug ethernet_up start_wpa "$INTERFACE" "$WPA_CONF" "$WPA_OPTS"
@@ -49,7 +49,7 @@ ethernet_up() {
             fi
         else
             # Clear remaining pid files. 
-            rm -f "/var/run/dhcpcd-${INTERFACE}".{pid,cache} >/dev/null 2>&1
+            rm -f "/var/run/dhcpcd-$INTERFACE".{pid,cache} >/dev/null 2>&1
             # If using own dns, tell dhcpcd to NOT replace resolv.conf
             [[ -n "$DNS1" || -n "$DNS" ]] && DHCP_OPTIONS="-C resolv.conf $DHCP_OPTIONS"
             # Start dhcpcd
@@ -86,7 +86,7 @@ ethernet_up() {
         fi
         ;;
       *)
-        report_fail "IP=\"\" must be either 'dhcp' or 'static'."
+        report_fail "IP must be either 'dhcp' or 'static'."
         return 1
         ;;
     esac
@@ -95,13 +95,13 @@ ethernet_up() {
     if [[ -n "$HOSTNAME" ]]; then
         report_debug ethernet_up hostname "$HOSTNAME"
         if ! hostname "$HOSTNAME"; then
-            report_fail "Setting hostname failed."
+            report_fail "Setting hostname $HOSTNAME failed."
             return 1
         fi
     fi
 
     # Generate a new resolv.conf
-    if [[ -n "$DNS1" ]] || [[ -n "$DNS" ]]; then
+    if [[ -n "$DNS1" || -n "$DNS" ]]; then
 
         : >/etc/resolv.conf 
         [[ -n "$DOMAIN" ]] && echo "domain $DOMAIN"   >>/etc/resolv.conf
diff --git a/src/connections/ethernet-iproute b/src/connections/ethernet-iproute
index 385e128..eedda15 100644
--- a/src/connections/ethernet-iproute
+++ b/src/connections/ethernet-iproute
@@ -20,12 +20,13 @@ ethernet_up() {
     report_debug ethernet_iproute_up ifup
     set_interface up "$INTERFACE"
 
+
     if ip link show dev "$INTERFACE" | fgrep -q "NO-CARRIER"; then
        report_iproute "No connection"
     fi
      
     if checkyesno "${AUTH8021X:-no}"; then 
-        . "${SUBR_DIR}/8021x"
+        . "$SUBR_DIR/8021x"
         [[ -z "$WPA_CONF" ]] && WPA_CONF="/etc/wpa_supplicant.conf"
         [[ -z "$WPA_OPTS" ]] && WPA_OPTS="-Dwired"
 
@@ -47,7 +48,7 @@ ethernet_up() {
     case "$IP" in
     dhcp)
         # Clear remaining pid files. 
-        rm -f "/var/run/dhcpcd-${INTERFACE}".{pid,cache} >/dev/null 2>&1
+        rm -f "/var/run/dhcpcd-$INTERFACE".{pid,cache} >/dev/null 2>&1
 
         # If using own dns, tell dhcpcd to NOT replace resolv.conf
         [[ -n "$DNS" ]] && DHCP_OPTIONS="-C resolv.conf $DHCP_OPTIONS"
@@ -61,7 +62,7 @@ ethernet_up() {
     static)
         if [[ -n "$ADDR" ]]; then
             report_debug ethernet_iproute_up ip addr add "$ADDR/24" brd + dev "$INTERFACE"
-            if ! ip addr add "${ADDR}/24" brd + dev "$INTERFACE"; then
+            if ! ip addr add "$ADDR/24" brd + dev "$INTERFACE"; then
                 report_iproute "Could not configure interface"
             fi
         fi
@@ -73,7 +74,7 @@ ethernet_up() {
         fi
         ;;
       *)
-        report_iproute "Profile error: IP must be either 'dhcp' or 'static'"
+        report_iproute "IP must be either 'dhcp' or 'static'"
         ;;
     esac
 
@@ -91,7 +92,7 @@ ethernet_up() {
     if [[ -n "$HOSTNAME" ]]; then
         report_debug ethernet_iproute_up hostname "$HOSTNAME"
         if ! hostname "$HOSTNAME"; then
-            report_iproute "Cannot set hostname"
+            report_iproute "Cannot set hostname to $HOSTNAME"
         fi
     fi
 
diff --git a/src/connections/wireless b/src/connections/wireless
index d341e99..2951cdd 100644
--- a/src/connections/wireless
+++ b/src/connections/wireless
@@ -19,8 +19,8 @@ wireless_up() {
     load_profile "$1"
     [[ -n "$2" ]] && ESSID="$2"              # JP: use the literal ESSID (though currently we only interpret wireless-dbus ESSIDs as regexps)
 
-    . "${SUBR_DIR}/8021x"
-    . "${SUBR_DIR}/wireless"
+    . "$SUBR_DIR/8021x"
+    . "$SUBR_DIR/wireless"
 
     # If rfkill is specified, enable device.
     if [[ -n "$RFKILL_NAME" ]]; then
@@ -33,7 +33,7 @@ wireless_up() {
     fi
 
     # Check if interface exists
-    if [[ ! -e /sys/class/net/"$INTERFACE" ]]; then
+    if [[ ! -e "/sys/class/net/$INTERFACE" ]]; then
         if ! echo "$INTERFACE" | fgrep -q ":"; then
             report_fail "interface $INTERFACE does not exist"
             return 1
@@ -79,7 +79,7 @@ wireless_up() {
     fi
   
     # Manually set iwconfig options
-    if [[ "$IWCONFIG" ]]; then
+    if [[ -n "$IWCONFIG" ]]; then
         report_debug wireless_up iwconfig "$INTERFACE" $IWCONFIG        
         iwconfig "$INTERFACE" $IWCONFIG
     fi
@@ -113,7 +113,7 @@ wireless_up() {
         fi
         
         report_debug wireless_up iwconfig "$INTERFACE" $WEP_OPTS
-        if ! eval iwconfig $INTERFACE $WEP_OPTS; then
+        if ! eval "iwconfig \"$INTERFACE\" $WEP_OPTS"; then
             report_fail "Could not set wireless configuration."
             return 1
         fi
@@ -149,6 +149,8 @@ wireless_up() {
         # Generate configuration
         if [[ "${#KEY}" -eq 64 ]]; then
             echo -e "network={ \nssid=\"$ESSID\" \npsk=$KEY \n}">> "$WPA_CONF/wpa.conf"
+            # JP: formerly I had  { \nssid=\"$ESSID\" \nproto=WPA \nkey_mgmt=WPA-PSK \npsk=$KEY \n}
+            # JP: is what's above better?
         elif ! echo "$KEY" | wpa_passphrase "$ESSID" >> "$WPA_CONF/wpa.conf"; then
             report_fail "Configuration generation failed."
             cat "$WPA_CONF/wpa.conf" >&2
@@ -169,7 +171,7 @@ wireless_up() {
         fi
         ;;
     wpa-config)
-        . "${SUBR_DIR}/8021x"
+        . "$SUBR_DIR/8021x"
         [[ -z "$WPA_CONF" ]] && WPA_CONF="/etc/wpa_supplicant.conf" # defaults
         [[ -z "$WPA_OPTS" ]] && WPA_OPTS="-Dwext"
         report_debug wireless_up start_wpa "$INTERFACE" "$WPA_CONF" "$WPA_OPTS"
@@ -187,7 +189,7 @@ wireless_up() {
     
     conn=ethernet
     checkyesno "${IPROUTE:-no}" && conn=ethernet-iproute
-    if ! "${CONN_DIR}/$conn" up "$1"; then
+    if ! "$CONN_DIR/$conn" up "$1"; then
         wireless_down "$1" YES
         return 1
     fi
@@ -196,7 +198,7 @@ wireless_up() {
 wireless_down() {
     local PROFILE="$1" NOETHERNETDOWN="$2"
     load_profile "$PROFILE"
-    . "${SUBR_DIR}/8021x"
+    . "$SUBR_DIR/8021x"
     if ! checkyesno "$NOETHERNETDOWN"; then
         conn=ethernet
         checkyesno "${IPROUTE:-no}" && conn=ethernet-iproute
diff --git a/src/net-profiles b/src/net-profiles
index ba75d56..a60d30b 100644
--- a/src/net-profiles
+++ b/src/net-profiles
@@ -18,7 +18,7 @@ case "$1" in
         done
 
         # $NET env var is passed from the kernel boot line
-        [ ! "$NETWORKS_MENU_TIMEOUT" ] && NETWORKS_MENU_TIMEOUT=5 
+        [[ -z "$NETWORKS_MENU_TIMEOUT" ]] && NETWORKS_MENU_TIMEOUT=5 
         
         if [[ "$NET" = "menu" ]]; then
             if /usr/bin/netcfg-menu "$NETWORKS_MENU_TIMEOUT"; then
@@ -26,7 +26,7 @@ case "$1" in
                 add_daemon net-profiles
                 exit 0
             fi
-        elif [[ "$NET" ]]; then
+        elif [[ -n "$NET" ]]; then
             if /usr/bin/netcfg2 -c "$NET"; then
                 echo "$NET" > "$STATE_DIR/net-profiles"        # JP: user may want to disconnect profile by calling net-profiles stop
                 add_daemon net-profiles
@@ -45,7 +45,7 @@ case "$1" in
                         break   # if netcfg-menu was called but failed: exit for loop
                         ;;
                     *) # Either interface or profile
-                        if [ "$network" = "${network#!}" ]; then # otherwise profile
+                        if [[ "$network" = "${network#!}" ]]; then # otherwise profile
                             if /usr/bin/netcfg2 -c "$network"; then
                                 echo "$network" > "$STATE_DIR/net-profiles"       # JP: user may want to disconnect profile by calling net-profiles stop
                                 add_daemon net-profiles
@@ -66,7 +66,7 @@ case "$1" in
 
         # shutdown any profiles started by netcfg (or from NET_PROFILES in rc.conf)
         # JP: only attempt to disconnect the profiles _this daemon_ was told to control
-        cat "${state_DIR}/net-profiles" 2>/dev/null | xargs -d'\n' /usr/bin/netcfg2 down  # JP: use xargs in case any of the profile names contain spaces etc
+        cat "$STATE_DIR/net-profiles" 2>/dev/null | xargs -d'\n' /usr/bin/netcfg2 down  # JP: use xargs in case any of the profile names contain spaces etc
         rm -f "$STATE_DIR/net-profiles"
         rm_daemon net-profiles
         ;;
diff --git a/src/net-rename b/src/net-rename
index c8927ed..d886daa 100644
--- a/src/net-rename
+++ b/src/net-rename
@@ -20,5 +20,6 @@ case "$1" in
     ;;
   *)
     exit_stderr "Usage: $0 {start|stop|restart}"
+    ;;
 esac
 # vim: set ts=4 et sw=4:
diff --git a/src/netcfg b/src/netcfg
index 197848a..1215332 100644
--- a/src/netcfg
+++ b/src/netcfg
@@ -86,7 +86,7 @@ case "$1" in
     -*|--*) 
         usage;;
     *)  
-        if [ -n "$1" ]; then
+        if [[ -n "$1" ]]; then
             profile_up "$1"
         else
             usage
diff --git a/src/netcfg-menu b/src/netcfg-menu
index c5e5324..2736ee1 100644
--- a/src/netcfg-menu
+++ b/src/netcfg-menu
@@ -10,26 +10,26 @@ i=0
 # JP: change for prof to while read prof to avoid assumption that profile names are always single tokens (no spaces etc.)
 while read prof; do
     # if there is a profile called "main", Use as default
-    [ "$prof" = "main" ] && DEFAULT=$prof
+    [ "$prof" = "main" ] && DEFAULT="main"
     unset DESCRIPTION           # JP: we can''t sandbox the sourced profiles, because we need to expose profiles[]
     . "$PROFILE_DIR/$prof"
     profiles[$i]="$prof"
-    i=$((i+1))
+    let i++
     profiles[$i]="$DESCRIPTION" # JP: this will usually have spaces and must be quoted
-    i=$((i+1))
+    let i++
 done < <(list_profiles | sort)  # JP: re-use list_profiles instead of duplicating it; avoid subshell we'd get by piping it to the while read...
 
-if [ ${#profiles} -eq 0 ]; then
+if [[ ${#profiles} -eq 0 ]]; then
     exit_err "No profiles were found in $PROFILE_DIR"
 fi
 
-[ "$NETWORKS_MENU_DEFAULT" ] && DEFAULT="$NETWORKS_MENU_DEFAULT"
+[[ -n "$NETWORKS_MENU_DEFAULT" ]] && DEFAULT="$NETWORKS_MENU_DEFAULT"
 # if no default yet, use the first entry
-[ "$DEFAULT" = "" ] && DEFAULT="${profiles[0]}"
+[[ -z "$DEFAULT" ]] && DEFAULT="${profiles[0]}"
 ANSWER=$(mktemp --tmpdir menu.XXXXXXXX) || exit 1
 
 # Set timeout
-if [ "$1" = "" ]; then
+if [ -z "$1" ]]; then
     TIMEOUT="0"
 else
     TIMEOUT="$1"
diff --git a/src/network b/src/network
index 7009893..e2cdef6 100644
--- a/src/network
+++ b/src/network
@@ -93,7 +93,7 @@ all_resume()
     done
 }
 
-# profile_up profile
+# profile_up profile [literal essid]
 #   put all profiles up
 #
 profile_up()
@@ -102,11 +102,15 @@ profile_up()
     # Keep inside subshell so that options from one profile don't cross to others
     # exit 1 used in a subshell is effectively exiting a new process
     [[ ! -d "$STATE_DIR" ]] && mkdir -p "$STATE_DIR"/{interfaces,profiles,suspend}
+
 	local PROFILE="$1"	# save PROFILE in a variable so that it's available to PRE_UP/POST_DOWN etc hooks
     
     load_profile "$PROFILE" || exit 1
 
-    check_profile "$PROFILE" && report_fail "$PROFILE already connected" &&  exit 1
+    if check_profile "$PROFILE"; then
+		report_fail "$PROFILE already connected"
+		exit 1
+	fi
 
     # NETWORKS_EXCLUSIVE, rc.conf: Profiles are globally mutually exclusive
     # EXCLUSIVE, network.d/profile: Individual profile is mutually exclusive
@@ -116,13 +120,13 @@ profile_up()
 
     report_try "$PROFILE up"
     
-    case $(check_iface $INTERFACE) in
+    case $(check_iface "$INTERFACE") in
         up)
-            if checkyesno $CHECK; then
+            if checkyesno "$CHECK"; then
                 report_fail "Interface $INTERFACE already in use"
                 exit 1
             else
-                interface_down $INTERFACE || exit 1
+                interface_down "$INTERFACE" || exit 1
                 load_profile "$PROFILE"
             fi
         ;;
@@ -223,19 +227,19 @@ profile_down()
 # Check if variable is a member of an array
 inarray()
 {
-search="$1"
-shift 
-for item in "$@"; do
-    if [[ "$item" == "$search" ]]; then
-        return 0
-    fi
-done
-return 1
+	local item search="$1"
+	shift 
+	for item in "$@"; do
+		if [[ "$item" == "$search" ]]; then
+			return 0
+		fi
+	done
+	return 1
 }
 
 quirk() {
-inarray "$1" "${QUIRKS[@]}"
-return $?
+	inarray "$1" "${QUIRKS[@]}"
+	return $?
 }
 
 # interface_down interface
@@ -375,7 +379,7 @@ set_interface()
 #
 checkyesno()
 {
-    local _value="${1}"
+    local _value="$1"
     #debug "checkyesno: $1 is set to $_value."
     case "$_value" in