netdev - Re: [PATCH v2 bpf-next 3/3] bpftool: add bash completions for btf command

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <b2eda1ce-4362-4b2a-0c60-7815dd8bb8d4@netronome.com>
Date:   Thu, 25 Apr 2019 12:15:21 +0100
From:   Quentin Monnet <quentin.monnet@...ronome.com>
To:     andrii.nakryiko@...il.com, kernel-team@...com,
        netdev@...r.kernel.org, bpf@...r.kernel.org, ast@...com,
        daniel@...earbox.net, yhs@...com, songliubraving@...com,
        kafai@...com, acme@...nel.org
Cc:     Andrii Nakryiko <andriin@...com>
Subject: Re: [PATCH v2 bpf-next 3/3] bpftool: add bash completions for btf
 command

2019-04-24 22:03 UTC-0700 ~ <andrii.nakryiko@...il.com>
> From: Andrii Nakryiko <andriin@...com>
> 
> Add full support for btf command in bash-completion script.
> 
> Cc: Quentin Monnet <quentin.monnet@...ronome.com>
> Cc: Yonghong Song <yhs@...com>
> Cc: Daniel Borkmann <daniel@...earbox.net>
> Cc: Alexei Starovoitov <ast@...com>
> Signed-off-by: Andrii Nakryiko <andriin@...com>
> ---
>  tools/bpf/bpftool/bash-completion/bpftool | 46 +++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
> 
> diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
> index 9f3ffe1e26ab..030f81bdec6a 100644
> --- a/tools/bpf/bpftool/bash-completion/bpftool
> +++ b/tools/bpf/bpftool/bash-completion/bpftool
> @@ -217,6 +217,7 @@ _bpftool()
>      done
>      cur=${words[cword]}
>      prev=${words[cword - 1]}
> +    pprev=${words[cword - 2]}
>  
>      local object=${words[1]} command=${words[2]}
>  
> @@ -607,6 +608,51 @@ _bpftool()
>                      ;;
>              esac
>              ;;
> +        btf)
> +            local PROG_TYPE='id pinned tag'
> +            local MAP_TYPE='id pinned'
> +            case $command in
> +                dump)
> +                    case $prev in
> +                        $command)
> +                            COMPREPLY+=( $( compgen -W "id map prog file" -- \
> +                                "$cur" ) )
> +                            return 0
> +                            ;;
> +                        prog)
> +                            COMPREPLY=( $( compgen -W "$PROG_TYPE" -- "$cur" ) )
> +                            return 0
> +                            ;;
> +                        map)
> +                            COMPREPLY=( $( compgen -W "$MAP_TYPE" -- "$cur" ) )
> +                            return 0
> +                            ;;
> +                        id)
> +                            case $pprev in
> +                                prog)
> +                                    _bpftool_get_prog_ids
> +                                    ;;
> +                                map)
> +                                    _bpftool_get_map_ids
> +                                    ;;
> +                            esac
> +                            return 0
> +                            ;;
> +                    *)
> +                        if _bpftool_search_list 'map'; then
> +                            COMPREPLY+=( $( compgen -W 'key value kv all' -- \
> +                                "$cur" ) )

Hi Andrii,

This COMPREPLY will suggest completion for "key|value|kv|all", even if
one of those words has been used on the command line before already (I
do not believe this is expected?). What about the following instead?

                            _bpftool_one_of_list 'key value kv all'

> +                        fi
> +                        return 0
> +                        ;;

Nit: It seems that the last bloc (the case when $prev matches on "*") is
not correctly indented, it should be aligned with $command/prog/map/id?

Other than this the completion seems good to me.

But note that I did not receive all of your patches, only the cover
letters (for v1 and v2 of the set) and this one (because I'm directly
CC-ed?). It looks like the other patches for both v1 and v2 did not make
it to the mailing lists or to patchwork, so you might want to double
check and resend if necessary.

Best,
Quentin

> +                    esac
> +                    ;;
> +                *)
> +                    [[ $prev == $object ]] && \
> +                        COMPREPLY=( $( compgen -W 'dump help' -- "$cur" ) )
> +                    ;;
> +            esac
> +            ;;
>          cgroup)
>              case $command in
>                  show|list)
>