[OpenWrt-Devel] [PATCHv3] ubox: run init script through shellcheck

[mail at adrianschmutzler.de]

Hi,

> -----Original Message-----
> From: openwrt-devel [mailto:openwrt-devel-bounces at lists.openwrt.org]
> On Behalf Of Rosen Penev
> Sent: Mittwoch, 15. April 2020 01:37
> To: openwrt-devel at lists.openwrt.org
> Subject: [OpenWrt-Devel] [PATCHv3] ubox: run init script through shellcheck
> 
> Warnings fixed:
> 
> SC2004: $/${} is unnecessary on arithmetic variables.
> SC2166: Prefer [ p ] && [ q ] as [ p -a q ] is not well defined.
> SC2086: Double quote to prevent globbing and word splitting.
> 
> Removed most usages of ${} with $. There's no need for it. ${} is mainly
> useful with substrings and arrays, which are not used here.
> 
> Signed-off-by: Rosen Penev <rosenp at gmail.com>
> ---
>  v3: Added quoting fixes.
>  v2: Fixed mistake with executing PIDCOUNT.
>  package/system/ubox/Makefile       |  2 +-
>  package/system/ubox/files/log.init | 32 +++++++++++++++---------------
>  2 files changed, 17 insertions(+), 17 deletions(-)
> 
> diff --git a/package/system/ubox/Makefile
> b/package/system/ubox/Makefile index cfa0b594e4..3970a4fc9c 100644
> --- a/package/system/ubox/Makefile
> +++ b/package/system/ubox/Makefile
> @@ -1,7 +1,7 @@
>  include $(TOPDIR)/rules.mk
> 
>  PKG_NAME:=ubox
> -PKG_RELEASE:=3
> +PKG_RELEASE:=4
> 
>  PKG_SOURCE_PROTO:=git
>  PKG_SOURCE_URL=$(PROJECT_GIT)/project/ubox.git
> diff --git a/package/system/ubox/files/log.init
> b/package/system/ubox/files/log.init
> index 250f805b44..4873a24472 100644
> --- a/package/system/ubox/files/log.init
> +++ b/package/system/ubox/files/log.init
> @@ -32,19 +32,19 @@ validate_log_daemon()
> 
>  start_service_daemon()
>  {
> -	[ $log_buffer_size -eq 0 -a $log_size -gt 0 ] &&
> log_buffer_size=$log_size
> -	[ $log_buffer_size -eq 0 ] && log_buffer_size=64
> +	[ "$log_buffer_size" -eq 0 ] && [ "$log_size" -gt 0 ] &&

I'm never sure whether a comparison [ "$string" -eq 0 ], i.e. one with quotes and one without using -eq works as expected in all cases.
I typically use [ "$string" = "0" ] instead, but I'm not sure whether that's effectively just the same.

Rest seems fine, despite some similar cases with -eq/-ne below.

Best

Adrian

> log_buffer_size="$log_size"
> +	[ "$log_buffer_size" -eq 0 ] && log_buffer_size=64
>  	procd_open_instance
>  	procd_set_param command "/sbin/logd"
> -	procd_append_param command -S "${log_buffer_size}"
> +	procd_append_param command -S "$log_buffer_size"
>  	procd_set_param respawn 5 1 -1
>  	procd_close_instance
>  }
> 
>  start_service_file()
>  {
> -	PIDCOUNT="$(( ${PIDCOUNT} + 1))"
> -	local pid_file="/var/run/logread.${PIDCOUNT}.pid"
> +	PIDCOUNT=$((PIDCOUNT + 1))
> +	local pid_file="/var/run/logread.$PIDCOUNT.pid"
> 
>  	[ "$2" = 0 ] || {
>  		echo "validation failed"
> @@ -52,34 +52,34 @@ start_service_file()
>  	}
>  	[ -z "${log_file}" ] && return
> 
> -	mkdir -p "$(dirname "${log_file}")"
> +	mkdir -p "$(dirname "$log_file")"
> 
>  	procd_open_instance
>  	procd_set_param command "$PROG" -f -F "$log_file" -p "$pid_file"
> -	[ -n "${log_size}" ] && procd_append_param command -S "$log_size"
> +	[ -n "$log_size" ] && procd_append_param command -S "$log_size"
>  	procd_close_instance
>  }
> 
>  start_service_remote()
>  {
> -	PIDCOUNT="$(( ${PIDCOUNT} + 1))"
> -	local pid_file="/var/run/logread.${PIDCOUNT}.pid"
> +	PIDCOUNT=$((PIDCOUNT + 1))
> +	local pid_file="/var/run/logread.$PIDCOUNT.pid"
> 
>  	[ "$2" = 0 ] || {
>  		echo "validation failed"
>  		return 1
>  	}
> -	[ "${log_remote}" -ne 0 ] || return
> -	[ -z "${log_ip}" ] && return
> -	[ -z "${log_hostname}" ] && log_hostname=$(cat
> /proc/sys/kernel/hostname)
> +	[ "$log_remote" -ne 0 ] || return
> +	[ -z "$log_ip" ] && return
> +	[ -z "$log_hostname" ] && log_hostname=$(cat
> +/proc/sys/kernel/hostname)
> 
>  	procd_open_instance
> -	procd_set_param command "$PROG" -f -h "$log_hostname" -r
> "$log_ip" "${log_port}" -p "$pid_file"
> -	case "${log_proto}" in
> +	procd_set_param command "$PROG" -f -h "$log_hostname" -r
> "$log_ip" "$log_port" -p "$pid_file"
> +	case "$log_proto" in
>  		"udp") procd_append_param command -u;;
> -		"tcp") [ "${log_trailer_null}" -eq 1 ] && procd_append_param
> command -0;;
> +		"tcp") [ "$log_trailer_null" -eq 1 ] && procd_append_param
> command
> +-0;;
>  	esac
> -	[ -z "${log_prefix}" ] || procd_append_param command -P
> "${log_prefix}"
> +	[ -z "$log_prefix" ] || procd_append_param command -P
> "$log_prefix"
>  	procd_close_instance
>  }
> 
> --
> 2.25.2
> 
> 
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel at lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: openpgp-digital-signature.asc
Type: application/pgp-signature
Size: 834 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/openwrt-devel/attachments/20200417/7a086e95/attachment.sig>
-------------- next part --------------
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel