[OpenWrt-Devel] [PATCHv3] ubox: run init script through shellcheck
Daniel Golle
daniel at makrotopia.org
Fri Apr 17 05:24:36 EDT 2020
On Fri, Apr 17, 2020 at 10:50:32AM +0200, mail at adrianschmutzler.de wrote:
> 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.
It's not:
i="0 -eq 0 -o 1"
[ $i -eq 1 ] && echo 1
1
[ "$i" -eq 0 ] && echo 1
bash: [: 0 -eq 0 -o 0: integer expression expected
So as a general rule, it does make sense to use quotes.
>
> 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
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel at lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
More information about the openwrt-devel
mailing list