Re: [PATCH bpf-next 1/7] tools: bpftool: slightly ease bash completion updates

On Fri, Jul 30, 2021 at 2:47 PM Quentin Monnet [off-list ref] wrote:

2021-07-30 11:45 UTC-0700 ~ Andrii Nakryiko [off-list ref]

quoted

On Thu, Jul 29, 2021 at 9:29 AM Quentin Monnet [off-list ref] wrote:

quoted

Bash completion for bpftool gets two minor improvements in this patch.

Move the detection of attach types for "bpftool cgroup attach" outside
of the "case/esac" bloc, where we cannot reuse our variable holding the
list of supported attach types as a pattern list. After the change, we
have only one list of cgroup attach types to update when new types are
added, instead of the former two lists.

Also rename the variables holding lists of names for program types, map
types, and attach types, to make them more unique. This can make it
slightly easier to point people to the relevant variables to update, but
the main objective here is to help run a script to check that bash
completion is up-to-date with bpftool's source code.

Signed-off-by: Quentin Monnet <redacted>
---
 tools/bpf/bpftool/bash-completion/bpftool | 57 +++++++++++++----------
 1 file changed, 32 insertions(+), 25 deletions(-)

diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index cc33c5824a2f..b2e33a2d8524 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool

@@ -404,8 +404,10 @@ _bpftool()
                             return 0
                             ;;
                         5)
-                            COMPREPLY=( $( compgen -W 'msg_verdict stream_verdict \
-                                stream_parser flow_dissector' -- "$cur" ) )
+                            local BPFTOOL_PROG_ATTACH_TYPES='msg_verdict \
+                                stream_verdict stream_parser flow_dissector'
+                            COMPREPLY=( $( compgen -W \
+                                "$BPFTOOL_PROG_ATTACH_TYPES" -- "$cur" ) )
                             return 0
                             ;;
                         6)

@@ -464,7 +466,7 @@ _bpftool()

                     case $prev in
                         type)
-                            COMPREPLY=( $( compgen -W "socket kprobe \
+                            local BPFTOOL_PROG_LOAD_TYPES='socket kprobe \
                                 kretprobe classifier flow_dissector \
                                 action tracepoint raw_tracepoint \
                                 xdp perf_event cgroup/skb cgroup/sock \

@@ -479,8 +481,9 @@ _bpftool()
                                 cgroup/post_bind4 cgroup/post_bind6 \
                                 cgroup/sysctl cgroup/getsockopt \
                                 cgroup/setsockopt cgroup/sock_release struct_ops \
-                                fentry fexit freplace sk_lookup" -- \
-                                                   "$cur" ) )
+                                fentry fexit freplace sk_lookup'
+                            COMPREPLY=( $( compgen -W \
+                                "$BPFTOOL_PROG_LOAD_TYPES" -- "$cur" ) )

nit: this and similar COMPREPLY assignments now can be on a single line now, no?

It will go over 80 characters, but OK, it will probably be more readable
on a single line. I'll change for v2.

80 character rule was lifted a while ago. 100 is totally fine. And in
some special cases readability beats even 100, IMO.