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

fff-layer3-config: return error values in functions instead of terminating #260

Closed
fbl wants to merge 1 commits from fbl:testmodefix into master
pull from: fbl:testmodefix
merge into: freifunk-franken:master
Conversation 1 Commits 1 Files Changed 1 +11 -6
1 changed files with 11 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
src/packages/fff/fff-layer3-config/files/usr/sbin/configure-layer3
View File

@ -7,6 +7,7 @@
execute_subshell() {
if [ $# -ne 1 ]; then
echo "Usage:" "$0" "<function>"
return 1
fi
for script in /etc/layer3.d/*; do
@ -23,7 +24,7 @@ execute_subshell() {
if [ $? -ne 0 ]; then
echo
echo "Error when executing" "$1" "from" "$(basename "$script")"
exit 1
return 1
fi
done
}
@ -32,29 +33,33 @@ configure() {
echo "This script might remove existing vlans, interfaces, addresses, etc."
read -r -p "Do you really want to continue? (y/n) " response
if ! ( [ "$response" == "y" ] || [ "$response" == "Y" ] ); then
exit 1
return 1
fi
echo
execute_subshell configure
execute_subshell configure || return $?
exit 0
return 0
jkimmel commented 2022-07-20 19:09:28 +02:00
Review
Copy Link

Ich glaub es reicht einfach execute_subshell configure alleine da stehen zu lassen. Die beiden returns können weg. Die Funktion nimmt den Wert des letzten Befehls an.

#!/bin/sh

success() { true; }
failure() { false; }
success; echo $?
failure; echo $?
Ich glaub es reicht einfach `execute_subshell configure` alleine da stehen zu lassen. Die beiden `return`s können weg. Die Funktion nimmt den Wert des letzten Befehls an. ```sh #!/bin/sh success() { true; } failure() { false; } success; echo $? failure; echo $? ```
fbl commented 2022-07-20 19:12:42 +02:00
Review
Copy Link

Das ist richtig, ich habe mich aber bewusst für diese etwas explizitere Form entschieden, weil ich das nicht offensichtlich finde. Dann müsste man es kommentieren, und dann kann ichs auch gleich ausschreiben.

Wenn noch mal jemand Code dazwischen fügt, dann bekommt der Code nicht versehentlich return values, die er eigentlich nicht haben soll.

Das ist richtig, ich habe mich aber bewusst für diese etwas explizitere Form entschieden, weil ich das nicht offensichtlich finde. Dann müsste man es kommentieren, und dann kann ichs auch gleich ausschreiben. Wenn noch mal jemand Code dazwischen fügt, dann bekommt der Code nicht versehentlich return values, die er eigentlich nicht haben soll.
}
reload_services() {
execute_subshell reload
reload_config
return 0
jkimmel commented 2022-07-20 19:09:32 +02:00
Review
Copy Link

Brauchsts das?

Brauchsts das?
fbl commented 2022-07-20 19:13:42 +02:00
Review
Copy Link

Ja, sonst gibts als Rückgabewert den Exitstatus von reload_config.

Ja, sonst gibts als Rückgabewert den Exitstatus von reload_config.
}
apply_changes() {
execute_subshell apply
reload_services
exit 0
return 0
jkimmel commented 2022-07-20 19:09:34 +02:00
Review
Copy Link

Und das?

Und das?
fbl commented 2022-07-20 19:15:25 +02:00
Review
Copy Link

Da reload_services sowieso immer 0 als return value hat.. Könnte man weglassen (da der Rückgabewert sowieso nirgends geprüft wird sowieso), aber ich finde es aus dem gleichen Grund wie weiter oben schöner, wenn man es explizit stehen lässt.

Da reload_services sowieso immer 0 als return value hat.. Könnte man weglassen (da der Rückgabewert sowieso nirgends geprüft wird sowieso), aber ich finde es aus dem gleichen Grund wie weiter oben schöner, wenn man es explizit stehen lässt.
}
revert_changes() {
execute_subshell revert
exit 0
return 0
jkimmel commented 2022-07-20 19:09:36 +02:00
Review
Copy Link

Und hier?

Und hier?
}
keep_changes() {