search

openwrt / telephony

The telephony packages feed
104 stars 240 forks source link

Treewide: Use return in init scripts instead of exit #790

Closed micmac1 closed 1 year ago

micmac1 commented 1 year ago

Maintainer: @jslachta & me Compile tested: master ath79 Run tested: 22.03 ath79

Description: Hi all,

I saw the commit [1] in packages. It fixed a bunch of init scripts to use return instead of exit in shell functions. I think we should do the same.

Usually we use "exit 1" in "start_service()" in our procd init scripts. Changing this to return is the right thing to do. There is a difference, though. If we run into "exit 1" we exit everything and the return value of the init script is "1". When converted to "return 1" the init script returns "0". So you could be in for a surprise if anything checked that return value. But in the files we provide we don't do that (e.g. freeswitch hotplug script calls init script but doesn't check return value).

Also, in rtpproxy and kamailio we use "exit 1" in helper functions. That way if the called helper is not happy and does "exit 1" the whole init script stops with exit status 1. Replace "exit 1" with "return 1" and and this doesn't happen, the init script continues, the programs are initialized, but without some extra (but ill-formatted) configuration. So again, slight change in behavior. Although the user still gets an error message (e.g. "init script does not understand $type entry").

This could be adapted, but I don't think we'd want that added complexity. Anyway, up for discussion.

[1] https://github.com/openwrt/packages/commit/b1651c5d5444b990b58180a26d6e76779cbb88a9

jospezial commented 1 year ago

Changing this to exit

return?

micmac1 commented 1 year ago

Changing this to exit

return?

Oh yes, sorry. I'll correct that.

jslachta commented 1 year ago

Hi Sebastian (@micmac1), this makes sense and should be treewide for all OpenWrt packages. Thank you for proposing this on this repository.

micmac1 commented 1 year ago

Hi Sebastian @.***), this makes sense and should be treewide for all OpenWrt packages. Thank you for proposing this on this repository.

Thanks for your feedback Jiri. I'll wait till next weekend to see if anybody else wants to comment.

Have a good weekend! Seb