Paolo Abeni pabeni@redhat.com writes:
Hi,
On 10/9/24 14:06, Petr Machata wrote:
diff --git a/tools/testing/selftests/net/lib/sh/defer.sh b/tools/testing/selftests/net/lib/sh/defer.sh new file mode 100644 index 000000000000..8d205c3f0445 --- /dev/null +++ b/tools/testing/selftests/net/lib/sh/defer.sh @@ -0,0 +1,115 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0
+# map[(scope_id,track,cleanup_id) -> cleanup_command] +# track={d=default | p=priority} +declare -A __DEFER__JOBS
+# map[(scope_id,track) -> # cleanup_commands] +declare -A __DEFER__NJOBS
+# scope_id of the topmost scope. +__DEFER__SCOPE_ID=0
+__defer__ndefer_key() +{
- local track=$1; shift
Minor nit: IMHO the trailing shift is here a bit confusing: it let me think about other arguments, which are not really expected.
This is IMHO how a function header should look like:
function() { local foo=$1; shift local bar=$1; shift local baz=$1; shift
... }
Because it lets you reorder the arguments freely just by reordering the lines, copy argument subsets to other functions without risking forgetting / screwing up renumbering, etc. It's easy to parse visually as well. If the function uses "$@" as rest argument, it will contain the rest by default. It's just a very convenient notation overall. Vast majority of net/lib.sh and net/forwarding/lib.sh use this.
+__defer__schedule() +{
- local track=$1; shift
- local ndefers=$(__defer__ndefers $track)
- local ndefers_key=$(__defer__ndefer_key $track)
- local defer_key=$(__defer__defer_key $track $ndefers)
- local defer="$@"
- __DEFER__JOBS[$defer_key]="$defer"
- __DEFER__NJOBS[$ndefers_key]=$((${__DEFER__NJOBS[$ndefers_key]} + 1))
'${__DEFER__NJOBS[$ndefers_key]}' is actually '$ndefers', right? If so it would be better to reuse the avail variable.
I figured I would leave it all spelled out, because the left hand side needs to be, and having the same expression on both sides makes it clear that this is just an X++ sort of a deal.