Re: [Buildroot] [PATCH 3/3] package/ssdp-responder: fix warnings from check-package and shellcheck

[Joachim Wiberg <troglobit@gmail.com>]

Hi Thomas!

On Mon, Oct 31, 2022 at 21:49, Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote:
> On Mon, 31 Oct 2022 18:46:32 +0100
> Joachim Wiberg <troglobit@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

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot