freifunk-franken
/
firmware
9
7
Fork 13
Code Issues 48 Pull Requests 7 Releases 17 Wiki Activity

fff-vpn-select: improve logic for fastd start/stop #13

Closed
adschm wants to merge 1 commits from adschm/firmware:vpnlogic into master
pull from: adschm/firmware:vpnlogic
merge into: freifunk-franken:master
Conversation 0 Commits 1 Files Changed 2 +6 -4
2 changed files with 6 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/packages/fff/fff-vpn-select/Makefile
View File

@ -1,7 +1,7 @@
include $(TOPDIR)/rules.mk
PKG_NAME:=fff-vpn-select
PKG_RELEASE:=5
PKG_RELEASE:=6
PKG_BUILD_DIR:=$(BUILD_DIR)/$(PKG_NAME)

8
src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
View File

@ -52,10 +52,12 @@ if [ -s "$hoodfile" ]; then
# fastd start/stop for various situations
pidfile="/tmp/run/fastd.fff.pid"
if [ "$(ls /etc/fastd/fff/peers/* 2>/dev/null)" ]; then
([ -s "$pidfile" ] && [ -d "/proc/$(cat "$pidfile")" ]) || /etc/init.d/fastd start
if [ -s "$pidfile" ] && [ -d "/proc/$(cat "$pidfile")" ]; then
jkimmel marked this conversation as resolved Outdated
jkimmel commented 2020-12-15 12:59:29 +01:00
Outdated
Review
Copy Link

[ -s "$pidfile" ] && [ -d "/proc/$(cat "$pidfile")" ] sollte aequivalent zu [ -s "$pidfile" -a -d "/proc/$(cat "$pidfile")" ] sein und spart einen aufruf an test

`[ -s "$pidfile" ] && [ -d "/proc/$(cat "$pidfile")" ]` sollte aequivalent zu `[ -s "$pidfile" -a -d "/proc/$(cat "$pidfile")" ]` sein und spart einen aufruf an test
adschm commented 2020-12-15 13:04:05 +01:00
Outdated
Review
Copy Link

shellcheck sagt "Prefer [ p ] && [ q ] as [ p -a q ] is not well defined."

shellcheck sagt "Prefer [ p ] && [ q ] as [ p -a q ] is not well defined."
# Stop if service is running but no peers present
[ "$(ls /etc/fastd/fff/peers/* 2>/dev/null)" ] || /etc/init.d/fastd stop
jkimmel commented 2020-12-15 13:04:36 +01:00
Outdated
Review
Copy Link

Gibt es einen Grund

[ "$(ls /etc/fastd/fff/peers/* 2>/dev/null)" ] || ...

statt einfach direkt

ls /etc/fastd/fff/peers/* 2>/dev/null || ...

zu nutzen?

Gibt es einen Grund `[ "$(ls /etc/fastd/fff/peers/* 2>/dev/null)" ] || ...` statt einfach direkt `ls /etc/fastd/fff/peers/* 2>/dev/null || ...` zu nutzen?
adschm commented 2020-12-15 13:06:56 +01:00
Outdated
Review
Copy Link

Ich fand das auch ekelig, habe dann gegoogled und tatsächlich wird genau das bestehende als Optimum empfohlen. Bei ls ohne [] muss man ja noch den Output unterdrücken ... Außerdem bin ich mir nicht sicher, wie ls mit return values umgeht, wenn man wildcards verwendet ...

Ich fand das auch ekelig, habe dann gegoogled und tatsächlich wird genau das bestehende als Optimum empfohlen. Bei ls ohne [] muss man ja noch den Output unterdrücken ... Außerdem bin ich mir nicht sicher, wie ls mit return values umgeht, wenn man wildcards verwendet ...
adschm commented 2020-12-15 13:08:52 +01:00
Outdated
Review
Copy Link

ls /etc/fastd/fff/peers/* &>/dev/null || ...

scheint zu funktionieren, aber ob das jetzt besser ist?

`ls /etc/fastd/fff/peers/* &>/dev/null || ...` scheint zu funktionieren, aber ob das jetzt besser ist?
jkimmel commented 2020-12-15 13:10:32 +01:00
Outdated
Review
Copy Link

also $(...) wuerde meines wissens eine neue subshell aufmachen. Die wuerden wir uns mit dem Ansatz sparen.

also `$(...)` wuerde meines wissens eine neue subshell aufmachen. Die wuerden wir uns mit dem Ansatz sparen.
rohammer commented 2020-12-19 14:00:10 +01:00
Outdated
Review
Copy Link

Die Sach ist ganz simpel. ls wirft bei einem leeren Verzeichnis keinen Fehler. Mit * hinten dran bekommen wir den Fehler weil das globbing nicht funzt. D.h. nicht unbedingt, dass das Verzeichnis leer ist. Darum der Test auf "leere" Ausgabe von ls.
In unserem Fall koennte es sogar besser sein, den * wegzulassen.

Die Sach ist ganz simpel. ls wirft bei einem leeren Verzeichnis keinen Fehler. Mit * hinten dran bekommen wir den Fehler weil das globbing nicht funzt. D.h. nicht unbedingt, dass das Verzeichnis leer ist. Darum der Test auf "leere" Ausgabe von ls. In unserem Fall koennte es sogar besser sein, den * wegzulassen.
else
([ -s "$pidfile" ] && [ -d "/proc/$(cat "$pidfile")" ]) && /etc/init.d/fastd stop
# Start if service is not running but peers are present
[ "$(ls /etc/fastd/fff/peers/* 2>/dev/null)" ] && /etc/init.d/fastd start
fi
fi
exit 0