[Buildroot] [PATCH 3/3] package/ssdp-responder: fix warnings from check-package and shellcheck
Joachim Wiberg
troglobit at gmail.com
Tue Nov 1 07:30:58 UTC 2022
Hi Thomas!
On Mon, Oct 31, 2022 at 21:49, Thomas Petazzoni <thomas.petazzoni at bootlin.com> wrote:
> On Mon, 31 Oct 2022 18:46:32 +0100
> Joachim Wiberg <troglobit at gmail.com> wrote:
>> +cmd()
>> +{
>> + start-stop-daemon -q -p "$PIDFILE" -x "$DAEMON" "$@"
>> + status=$?
>> + [ $status -eq 0 ] && echo "OK" || echo "FAIL"
>> +
>> + return $status
>> +}
> I don't think we're using this cmd construct anywhere else in the tree,
> or did I miss some change in our coding style/policy?
I mentioned it in the cover letter, but that information should have
been here in this patch. Sorry about that.
It all started out with utils/check-package telling me I used $DAEMON
wrong. While changing that I ended up with a final comment from it
that said I should also "run shellcheck and fix the warnings".
It in turn had several grievances which I took one by one. In this one
I used the same construct as in package/smcroute/S41smcroute to work
around a warning about using `$?` instead of using an `if cmd; then ...`
>> start() {
>> - printf 'Starting %s: ' "$NAME"
>> - start-stop-daemon -S -q -p $PIDFILE -x $DAEMON -- $DAEMON_ARGS
>> - [ $? = 0 ] && echo "OK" || echo "FAIL"
>
> This all looked matching our coding style. Why are you changing this?
>> case "$1" in
>> - start|stop|restart)
>> - "$1"
>> - ;;
>> - reload)
>> - restart
>> - ;;
>> - *)
>> - echo "Usage: $0 {start|stop|restart|reload}"
>> - exit 1
>> + start|stop|restart)
>> + "$1"
>> + ;;
>> + reload)
>> + restart
>> + ;;
>> + *)
>> + echo "Usage: $0 {start|stop|restart|reload}"
>> + exit 1
>
> I'm not sure what is the recommended indentation style in our init
> scripts. Tabs? Spaces?
Shellcheck pointed out this section had four spaces indent instead of
the eight (tab) used for the rest of the script. Iirc from the "own
code coding style" discussions C and shell script should follow the
same style, but I may very well be wrong here.
Best regards
/Joachim
More information about the buildroot
mailing list