On Thu, Dec 12, 2019 at 1:12 PM Quentin Monnet
<quentin.monnet@xxxxxxxxxxxxx> wrote:
>
> 2019-12-12 08:41 UTC-0800 ~ Andrii Nakryiko <andriin@xxxxxx>
> > Add BASH completions for gen sub-command.
> >
> > Cc: Quentin Monnet <quentin.monnet@xxxxxxxxxxxxx>
> > Acked-by: Martin KaFai Lau <kafai@xxxxxx>
> > Signed-off-by: Andrii Nakryiko <andriin@xxxxxx>
> > ---
> > tools/bpf/bpftool/bash-completion/bpftool | 11 +++++++++++
> > tools/bpf/bpftool/main.c | 2 +-
> > tools/testing/selftests/bpf/prog_tests/skeleton.c | 6 ++++--
> > tools/testing/selftests/bpf/progs/test_skeleton.c | 3 ++-
> > 4 files changed, 18 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
> > index 70493a6da206..986519cc58d1 100644
> > --- a/tools/bpf/bpftool/bash-completion/bpftool
> > +++ b/tools/bpf/bpftool/bash-completion/bpftool
> > @@ -716,6 +716,17 @@ _bpftool()
> > ;;
> > esac
> > ;;
> > + gen)
> > + case $command in
> > + skeleton)
> > + _filedir
> > + ;;
> > + *)
> > + [[ $prev == $object ]] && \
> > + COMPREPLY=( $( compgen -W 'skeleton help' -- "$cur" ) )
> > + ;;
> > + esac
> > + ;;
>
> Hi Andrii,
>
> Bpftool completion looks OK to me...
>
> > cgroup)
> > case $command in
> > show|list|tree)
> > diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
> > index 758b294e8a7d..1fe91c558508 100644
> > --- a/tools/bpf/bpftool/main.c
> > +++ b/tools/bpf/bpftool/main.c
> > @@ -58,7 +58,7 @@ static int do_help(int argc, char **argv)
> > " %s batch file FILE\n"
> > " %s version\n"
> > "\n"
> > - " OBJECT := { prog | map | cgroup | perf | net | feature | btf }\n"
> > + " OBJECT := { prog | map | cgroup | perf | net | feature | btf | gen }\n"
>
> ... but this is part of the usage message, and ideally should be added
> when you add the new feature to bpftool in patch 11. Not a major issue,
> but ...
>
> > " " HELP_SPEC_OPTIONS "\n"
> > "",
> > bin_name, bin_name, bin_name);
> > diff --git a/tools/testing/selftests/bpf/prog_tests/skeleton.c b/tools/testing/selftests/bpf/prog_tests/skeleton.c
> > index d65a0203e1df..94e0300f437a 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/skeleton.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/skeleton.c
> > @@ -39,8 +39,10 @@ void test_skeleton(void)
> > CHECK(bss->out2 != 2, "res2", "got %lld != exp %d\n", bss->out2, 2);
> > CHECK(bss->out3 != 3, "res3", "got %d != exp %d\n", (int)bss->out3, 3);
> > CHECK(bss->out4 != 4, "res4", "got %lld != exp %d\n", bss->out4, 4);
> > - CHECK(bss->out5.a != 5, "res5", "got %d != exp %d\n", bss->out5.a, 5);
> > - CHECK(bss->out5.b != 6, "res6", "got %lld != exp %d\n", bss->out5.b, 6);
> > + CHECK(bss->handler_out5.a != 5, "res5", "got %d != exp %d\n",
> > + bss->handler_out5.a, 5);
> > + CHECK(bss->handler_out5.b != 6, "res6", "got %lld != exp %d\n",
> > + bss->handler_out5.b, 6);
>
> ... This and the code below does not seem to relate to bpftool
> completion at all. And it was not present in v1. I suspect this code was
> not intended to end up in this patch?
>
Oh yeah, I screwed up amending, sorry about that. I will fix both
issues, thanks.
> >
> > cleanup:
> > test_skeleton__destroy(skel);
> > diff --git a/tools/testing/selftests/bpf/progs/test_skeleton.c b/tools/testing/selftests/bpf/progs/test_skeleton.c
> > index 303a841c4d1c..db4fd88f3ecb 100644
> > --- a/tools/testing/selftests/bpf/progs/test_skeleton.c
> > +++ b/tools/testing/selftests/bpf/progs/test_skeleton.c
> > @@ -16,7 +16,6 @@ long long in4 __attribute__((aligned(64))) = 0;
> > struct s in5 = {};
> >
> > long long out2 = 0;
> > -struct s out5 = {};
> > char out3 = 0;
> > long long out4 = 0;
> > int out1 = 0;
> > @@ -25,6 +24,8 @@ int out1 = 0;
> > SEC("raw_tp/sys_enter")
> > int handler(const void *ctx)
> > {
> > + static volatile struct s out5;
> > +
> > out1 = in1;
> > out2 = in2;
> > out3 = in3;
> >
>
