Re: [PATCH bpf-next 1/5] selftests/bpf: fix btf_dedup testing code

[Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>]

Em Wed, Feb 27, 2019 at 02:46:37PM -0800, Andrii Nakryiko escreveu:
> btf_dedup testing code doesn't account for length of struct btf_header
> when calculating the start of a string section. This patch fixes this
> problem.
> 
> Fixes: 49b57e0d01db ("tools/bpf: remove btf__get_strings() superseded by raw data API")

I think this clarifies things, but a Fixes seems excessive, right? I.e.
if you missed it in both sides of the (a != b) expression, the test will
be just as valid.

I say this because Fixes tags are now tracked and generates backporting
efforts that sometimes end up causing unnecessary human exchanges when
the patches don't apply because of some other patch.o

Or is there some further use of 'test_strs' and 'expect_strs' further
down that do_test_dedup() function?

/me scratches head, probably missing something...

- Arnaldo

> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---
>  tools/testing/selftests/bpf/.gitignore | 1 +
>  tools/testing/selftests/bpf/test_btf.c | 4 ++--
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore
> index e47168d1257d..3b74d23fffab 100644
> --- a/tools/testing/selftests/bpf/.gitignore
> +++ b/tools/testing/selftests/bpf/.gitignore
> @@ -14,6 +14,7 @@ feature
>  test_libbpf_open
>  test_sock
>  test_sock_addr
> +test_sock_fields
>  urandom_read
>  test_btf
>  test_sockmap
> diff --git a/tools/testing/selftests/bpf/test_btf.c b/tools/testing/selftests/bpf/test_btf.c
> index 02d314383a9c..1426c0a905c8 100644
> --- a/tools/testing/selftests/bpf/test_btf.c
> +++ b/tools/testing/selftests/bpf/test_btf.c
> @@ -5936,9 +5936,9 @@ static int do_test_dedup(unsigned int test_num)
>  	}
>  
>  	test_hdr = test_btf_data;
> -	test_strs = test_btf_data + test_hdr->str_off;
> +	test_strs = test_btf_data + sizeof(*test_hdr) + test_hdr->str_off;
>  	expect_hdr = expect_btf_data;
> -	expect_strs = expect_btf_data + expect_hdr->str_off;
> +	expect_strs = expect_btf_data + sizeof(*test_hdr) + expect_hdr->str_off;
>  	if (CHECK(test_hdr->str_len != expect_hdr->str_len,
>  		  "test_hdr->str_len:%u != expect_hdr->str_len:%u",
>  		  test_hdr->str_len, expect_hdr->str_len)) {
> -- 
> 2.17.1

-- 

- Arnaldo