Small review of PPP related connections

The most important change is that we now bring the interface down
correctly in pppoe_down.

2 files changed, 40 insertions, 40 deletions

diff --git a/src/lib/connections/mobile_ppp b/src/lib/connections/mobile_ppp
index 19d63f9..d117367 100644
--- a/src/lib/connections/mobile_ppp
+++ b/src/lib/connections/mobile_ppp
@@ -3,11 +3,11 @@
 # Also see <https://wiki.archlinux.org/index.php/3G_and_GPRS_modems_with_pppd> for more information.
 
 : ${PPPD:=pppd}
-# Set the interface route
 : ${InterfaceRoot=dev/}
 
-_quotestring() {
-    echo "\"${1/\"/\\\"}\""
+quote_word() {
+    set -- "${@//\\/\\\\}"
+    printf '"%s"\n' "${@/\"/\\\"}"
 }
 
 mobile_ppp_up() {
@@ -18,11 +18,9 @@ mobile_ppp_up() {
     chmod 700 "$STATE_DIR/mobile_ppp.${Interface}.${Profile}/"
     cfg="$STATE_DIR/mobile_ppp.${Interface}.${Profile}/options"
     chat="$STATE_DIR/mobile_ppp.${Interface}.${Profile}/modem.chat"
-    : > "${cfg}"
+    echo "linkname $(quote_word "${Profile}")" > "${cfg}"
     chmod 600 "${cfg}"
 
-    echo "linkname $(_quotestring "${Profile}")" >> "${cfg}"
-
     cat >> "${cfg}" << EOF
 ${Interface}
 921600
@@ -53,8 +51,8 @@ EOF
     # Writes username and password
     echo "noauth" >> "${cfg}"
     echo "hide-password" >> ${cfg}
-    [[ -n ${User} ]] && echo "user $(_quotestring "${User}")" >> "${cfg}"
-    [[ -n ${Password} ]] && echo "password $(_quotestring "${Password}")" >> "${cfg}"
+    [[ -n ${User} ]] && echo "user $(quote_word "${User}")" >> "${cfg}"
+    [[ -n ${Password} ]] && echo "password $(quote_word "${Password}")" >> "${cfg}"
 
     #echo "'OK' @/etc/ppp/chatscripts/pin" >> "${chat}"
     if [ -n "${Pin}" ]; then
@@ -116,11 +114,7 @@ EOF
     # Add the chat script line to the configuration
     echo "connect \"/usr/sbin/chat -v -t15 -f ${chat}\"" >> "${cfg}"
 
-    ip link set dev "${Interface}" up
-    #$PPPD call "$Peer" updetach child-timeout "$PPPTimeout" linkname "$Peer"
-    $PPPD file "${cfg}"
-
-    if [[ $? -ne 0 ]]; then
+    if ! $PPPD file "${cfg}"; then
         rmdir "$STATE_DIR/mobile_ppp.${Interface}.${Profile}/"
         report_error "Couldn't make pppd connection."
         return 1
@@ -128,19 +122,17 @@ EOF
 }
 
 mobile_ppp_down() {
-    local cfg
-    local chat
+    local cfg chat pidfile pid
     cfg="$STATE_DIR/mobile_ppp.${Interface}.${Profile}/options"
     chat="$STATE_DIR/mobile_ppp.${Interface}.${Profile}/modem.chat"
-    PIDFILE="/var/run/ppp-${Profile}.pid"
+    pidfile="/var/run/ppp-${Profile}.pid"
 
-    if [[ -e $PIDFILE ]]; then
-        read PID < "$PIDFILE"
-        [[ "$PID" ]] && kill "$PID"
+    if [[ -e $pidfile ]]; then
+        read pid < "$pidfile"
+        [[ "$pid" ]] && kill "$pid"
     fi
 
-    rm "${cfg}"
-    rm "${chat}"
+    rm "${cfg}" "${chat}"
     rmdir "$STATE_DIR/mobile_ppp.${Interface}.${Profile}/"
 }
 
diff --git a/src/lib/connections/pppoe b/src/lib/connections/pppoe
index d392397..177f575 100644
--- a/src/lib/connections/pppoe
+++ b/src/lib/connections/pppoe
@@ -2,17 +2,27 @@
 
 : ${PPPD:=pppd}
 
-_quotestring() {
-    echo "\"${1/\"/\\\"}\""
+quote_word() {
+    set -- "${@//\\/\\\\}"
+    printf '"%s"\n' "${@/\"/\\\"}"
 }
 
 pppoe_up() {
     local cfg
 
+    if ! is_interface "$Interface"; then
+        report_error "Interface '$Interface' does not exist"
+        return 1
+    fi
+    if ! bring_interface_up "$Interface"; then
+        report_error "Failed to bring interface '$Interface' up"
+        return 1
+    fi
+
     mkdir -p "$STATE_DIR/pppoe.${Interface}.${Profile}/"
     chmod 700 "$STATE_DIR/pppoe.${Interface}.${Profile}/"
     cfg="$STATE_DIR/pppoe.${Interface}.${Profile}/options"
-    : > "${cfg}"
+    echo "linkname $(quote_word "${Profile}")" > "${cfg}"
     chmod 600 "${cfg}"
 
     echo "plugin rp-pppoe.so" >> "${cfg}"
@@ -26,7 +36,6 @@ pppoe_up() {
     if is_yes "${UsePeerDNS:-yes}"; then
         echo "usepeerdns" >> "${cfg}"
     fi
-    echo "linkname $(_quotestring "${Profile}")" >> "${cfg}"
     echo "maxfail 5" >> "${cfg}"
     echo "updetach" >> "${cfg}"
     if [[ ${ConnectionMode} == demand ]]; then
@@ -35,20 +44,17 @@ pppoe_up() {
     else
         echo "persist" >> "${cfg}"
     fi
-    echo "user $(_quotestring "${User}")" >> "${cfg}"
-    echo "password $(_quotestring "${Password}")" >> "${cfg}"
+    echo "user $(quote_word "${User}")" >> "${cfg}"
+    echo "password $(quote_word "${Password}")" >> "${cfg}"
     [[ -n ${LCPEchoInterval} ]] && echo "lcp-echo-interval ${LCPEchoInterval}" >> "${cfg}"
     [[ -n ${LCPEchoFailure} ]] && echo "lcp-echo-failure ${LCPEchoFailure}" >> "${cfg}"
-    [[ -n ${PPPoEService} ]] && echo "rp_pppoe_service $(_quotestring "${PPPoEService}")" >> "${cfg}"
-    [[ -n ${PPPoEAC} ]] && echo "rp_pppoe_ac $(_quotestring "${PPPoEAC}")" >> "${cfg}"
-    [[ -n ${PPPoESession} ]] && echo "rp_pppoe_sess $(_quotestring "${PPPoESession}")" >> "${cfg}"
-    [[ -n ${PPPoEMAC} ]] && echo "pppoe-mac $(_quotestring "${PPPoEMAC}")" >> "${cfg}"
+    [[ -n ${PPPoEService} ]] && echo "rp_pppoe_service $(quote_word "${PPPoEService}")" >> "${cfg}"
+    [[ -n ${PPPoEAC} ]] && echo "rp_pppoe_ac $(quote_word "${PPPoEAC}")" >> "${cfg}"
+    [[ -n ${PPPoESession} ]] && echo "rp_pppoe_sess $(quote_word "${PPPoESession}")" >> "${cfg}"
+    [[ -n ${PPPoEMAC} ]] && echo "pppoe-mac $(quote_word "${PPPoEMAC}")" >> "${cfg}"
     [[ ${PPPoEIP6} == yes ]] && echo "+ipv6" >> "${cfg}"
 
-    ip link set dev "${Interface}" up
-    $PPPD file "${cfg}"
-
-    if [[ $? -ne 0 ]]; then
+    if ! $PPPD file "${cfg}"; then
         rm "${cfg}"
         rmdir "$STATE_DIR/pppoe.${Interface}.${Profile}/"
         report_error "Couldn't make pppd connection."
@@ -57,17 +63,19 @@ pppoe_up() {
 }
 
 pppoe_down() {
-    local cfg
+    local cfg pidfile pid
     cfg="$STATE_DIR/pppoe.${Interface}.${Profile}/options"
-    PIDFILE="/var/run/ppp-${Profile}.pid"
+    pidfile="/var/run/ppp-${Profile}.pid"
 
-    if [[ -e $PIDFILE ]]; then
-        read PID < "$PIDFILE"
-        [[ "$PID" ]] && kill "$PID"
+    if [[ -e $pidfile ]]; then
+        read pid < "$pidfile"
+        [[ "$pid" ]] && kill "$pid"
     fi
 
     rm "${cfg}"
     rmdir "$STATE_DIR/pppoe.${Interface}.${Profile}/"
+
+    bring_interface_down "$Interface"
 }