sqm-scripts: package assumes stat -c support

[nwf]

Maintainer: @tohojo Environment: ipq806x, TP-Link Archer C2600, HEAD Description:

The stat calls in https://github.com/tohojo/sqm-scripts/blob/d22bfa87fbf9ead036ffc8d21e037bcb76944dab/src/functions.sh#L284-L293 invoke stat with -c, but the default busybox configuration has https://github.com/openwrt/openwrt/blob/5384c9337f2323727081e32369a86b62e72c47d8/package/utils/busybox/Config-defaults.in#L790-L792 and https://github.com/openwrt/openwrt/blob/5384c9337f2323727081e32369a86b62e72c47d8/package/utils/busybox/config/coreutils/Config.in#L694-L701 and so at boot I get this dumped into the logs:

Thu Oct 27 04:08:01 2022 daemon.notice procd: /etc/rc.d/S50sqm: stat: unrecognized option: c
Thu Oct 27 04:08:01 2022 daemon.notice procd: /etc/rc.d/S50sqm: BusyBox v1.35.0 (2022-10-21 19:33:35 UTC) multi-call binary.
Thu Oct 27 04:08:01 2022 daemon.notice procd: /etc/rc.d/S50sqm:
Thu Oct 27 04:08:01 2022 daemon.notice procd: /etc/rc.d/S50sqm: Usage: stat [-ltf] FILE...
Thu Oct 27 04:08:01 2022 daemon.notice procd: /etc/rc.d/S50sqm:
Thu Oct 27 04:08:01 2022 daemon.notice procd: /etc/rc.d/S50sqm: Display file (default) or filesystem status
Thu Oct 27 04:08:01 2022 daemon.notice procd: /etc/rc.d/S50sqm:
Thu Oct 27 04:08:01 2022 daemon.notice procd: /etc/rc.d/S50sqm:     -f  Display filesystem status
Thu Oct 27 04:08:01 2022 daemon.notice procd: /etc/rc.d/S50sqm:     -L  Follow links
Thu Oct 27 04:08:01 2022 daemon.notice procd: /etc/rc.d/S50sqm:     -t  Terse display
Thu Oct 27 04:08:01 2022 daemon.notice procd: /etc/rc.d/S50sqm: stat: unrecognized option: c
Thu Oct 27 04:08:01 2022 daemon.notice procd: /etc/rc.d/S50sqm: BusyBox v1.35.0 (2022-10-21 19:33:35 UTC) multi-call binary.
Thu Oct 27 04:08:01 2022 daemon.notice procd: /etc/rc.d/S50sqm:
Thu Oct 27 04:08:01 2022 daemon.notice procd: /etc/rc.d/S50sqm: Usage: stat [-ltf] FILE...
Thu Oct 27 04:08:01 2022 daemon.notice procd: /etc/rc.d/S50sqm:
Thu Oct 27 04:08:01 2022 daemon.notice procd: /etc/rc.d/S50sqm: Display file (default) or filesystem status
Thu Oct 27 04:08:01 2022 daemon.notice procd: /etc/rc.d/S50sqm:
Thu Oct 27 04:08:01 2022 daemon.notice procd: /etc/rc.d/S50sqm:     -f  Display filesystem status
Thu Oct 27 04:08:01 2022 daemon.notice procd: /etc/rc.d/S50sqm:     -L  Follow links
Thu Oct 27 04:08:01 2022 daemon.notice procd: /etc/rc.d/S50sqm:     -t  Terse display

I'm not sure how this {c,sh}ould be rectified in software, since there are at least two possible sources for the stat executable (busybox and GNU coreutils) and disjunctive dependencies are tricky. It may be best just to document it in the package description in the makefile?

[tohojo]

So just above the bit of code you linked to, we have this:

https://github.com/tohojo/sqm-scripts/blob/d22bfa87fbf9ead036ffc8d21e037bcb76944dab/src/functions.sh#L280-L282

    # OpenWrt doesn't have stat; for now just skip the remaining tests if it's
    # not available
    command -v stat >/dev/null 2>&1 || return 0

so I guess we can just amend that check to also look for the -c parameter...

[tohojo]

Could you please try applying this patch to functions.sh and see if that resolves the error?

diff --git a/src/functions.sh b/src/functions.sh
index a6ec614652d4..21efa260c12e 100644
--- a/src/functions.sh
+++ b/src/functions.sh
@@ -277,9 +277,10 @@ check_state_dir() {
         exit 1
     fi

-    # OpenWrt doesn't have stat; for now just skip the remaining tests if it's
-    # not available
-    command -v stat >/dev/null 2>&1 || return 0
+    # OpenWrt doesn't have stat by default, and if it does have it, stat is
+    # usually built without support for the -c parameter. Check for these and
+    # skip the remaining tests if we don't have a usable 'stat' binary.
+    (command -v stat && stat -c '%a' /) >/dev/null 2>&1 || return 0

     PERM="0$(stat -L -c '%a' "${SQM_STATE_DIR}")"
     if [ "$((PERM & 0002))" -ne 0 ]; then

[nwf]

I fixed it locally by having busybox build with support for stat -c. It certainly looks like it will quiet the errors, but are those tests really not that important?

[tohojo]

Nathaniel Wesley Filardo @.***> writes:

That certainly looks sensible to me, but I fixed it locally by having busybox build with support for stat -c. It certainly looks like it will quiet the errors, but are those tests really not that important?

Not really; it's running permission checks on the directories to make sure we don't leak data to other users, but openwrt runs everything as root anyway, so it's not a huge deal there...

[nwf]

In that case, I think your diff is a fine fix.