| Message ID | 20180809164911.2226-1-hacks@slashdirt.org |
|---|---|
| State | Superseded |
| Delegated to: | John Crispin |
| Headers | show |
| Series | [OpenWrt-Devel,v3] base-files: make wifi report unknown command | expand |
On 9 August 2018 at 18:49, Thibaut VARÈNE <hacks@slashdirt.org> wrote: > Avoid having /sbin/wifi silently ignore unknown keywords and execute > "enable"; instead display the help message and exit with an error. > > Spell out the 'enable' keyword and preserve the implicit assumption > that runing /sbin/wifi without argument performs "enable". > > Signed-off-by: Thibaut VARÈNE <hacks@slashdirt.org> > --- Please keep a changelog here to know what's changed between submissions. > package/base-files/files/sbin/wifi | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/package/base-files/files/sbin/wifi b/package/base-files/files/sbin/wifi > index 83befc0d6f..68536ed25f 100755 > --- a/package/base-files/files/sbin/wifi > +++ b/package/base-files/files/sbin/wifi > @@ -241,5 +241,6 @@ case "$1" in > reload) wifi_reload "$2";; > reload_legacy) wifi_reload_legacy "$2";; > --help|help) usage;; > - *) ubus call network reload; wifi_updown "enable" "$2";; > + ''|enable) ubus call network reload; wifi_updown "enable" "$2";; Not that I particularily want to bikeshed here, but the disable command is "down", so the enable command should be "up", not "enable". There are actually existing users of "up": see https://github.com/openwrt/openwrt/blob/master/package/network/config/netifd/files/sbin/ifup#L43 and https://github.com/openwrt/openwrt/blob/master/package/network/config/netifd/files/sbin/ifup#L75 or https://github.com/openwrt/openwrt/blob/master/package/base-files/files/etc/rc.button/rfkill#L30 so it isn't just a should, it's a must. Or the existing users must be changed before. > + *) usage; exit 1;; > esac Regards Jonas
> On 9 Aug 2018, at 19:58, Jonas Gorski <jonas.gorski@gmail.com> wrote: > > On 9 August 2018 at 18:49, Thibaut VARÈNE <hacks@slashdirt.org> wrote: >> Avoid having /sbin/wifi silently ignore unknown keywords and execute >> "enable"; instead display the help message and exit with an error. >> >> Spell out the 'enable' keyword and preserve the implicit assumption >> that runing /sbin/wifi without argument performs "enable". >> >> Signed-off-by: Thibaut VARÈNE <hacks@slashdirt.org> >> --- > > Please keep a changelog here to know what's changed between submissions. it’s a 2-liner patch. I don’t think it warrants a changelog in the commit message :) v4: use “up” instead of “enable” and add it to usage output. Cheers, T.
On 9 August 2018 at 20:35, Thibaut <hacks@slashdirt.org> wrote: > >> On 9 Aug 2018, at 19:58, Jonas Gorski <jonas.gorski@gmail.com> wrote: >> >> On 9 August 2018 at 18:49, Thibaut VARÈNE <hacks@slashdirt.org> wrote: >>> Avoid having /sbin/wifi silently ignore unknown keywords and execute >>> "enable"; instead display the help message and exit with an error. >>> >>> Spell out the 'enable' keyword and preserve the implicit assumption >>> that runing /sbin/wifi without argument performs "enable". >>> >>> Signed-off-by: Thibaut VARÈNE <hacks@slashdirt.org> >>> --- >> >> Please keep a changelog here to know what's changed between submissions. > > it’s a 2-liner patch. I don’t think it warrants a changelog in the commit message :) The fact that you needed to send 4 versions should be enough of an argument to keep a changelog regardless of the size of the patch ;) Also the changelog belongs below the tear-off line ("---") so it doesn't end in the commit message. So please keep that in mind for future patches. Regards Jonas
diff --git a/package/base-files/files/sbin/wifi b/package/base-files/files/sbin/wifi index 83befc0d6f..68536ed25f 100755 --- a/package/base-files/files/sbin/wifi +++ b/package/base-files/files/sbin/wifi @@ -241,5 +241,6 @@ case "$1" in reload) wifi_reload "$2";; reload_legacy) wifi_reload_legacy "$2";; --help|help) usage;; - *) ubus call network reload; wifi_updown "enable" "$2";; + ''|enable) ubus call network reload; wifi_updown "enable" "$2";; + *) usage; exit 1;; esac
Avoid having /sbin/wifi silently ignore unknown keywords and execute "enable"; instead display the help message and exit with an error. Spell out the 'enable' keyword and preserve the implicit assumption that runing /sbin/wifi without argument performs "enable". Signed-off-by: Thibaut VARÈNE <hacks@slashdirt.org> --- package/base-files/files/sbin/wifi | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)