Re: [PATCH bpf-next 06/11] selftests/bpf: add checking of raw type dump in BTF writer APIs selftests

[Song Liu <songliubraving@fb.com>]

> On Oct 28, 2020, at 5:58 PM, Andrii Nakryiko <andrii@kernel.org> wrote:
> 
> From: Andrii Nakryiko <andriin@fb.com>
> 
> Add re-usable btf_helpers.{c,h} to provide BTF-related testing routines. Start
> with adding a raw BTF dumping helpers.
> 
> Raw BTF dump is the most succinct and at the same time a very human-friendly
> way to validate exact contents of BTF types. Cross-validate raw BTF dump and
> writable BTF in a single selftest. Raw type dump checks also serve as a good
> self-documentation.
> 
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>

Acked-by: Song Liu <songliubraving@fb.com>

with a couple nits:

[...]

> +
> +/* Print raw BTF type dump into a local buffer and return string pointer back.
> + * Buffer *will* be overwritten by subsequent btf_type_raw_dump() calls
> + */
> +const char *btf_type_raw_dump(const struct btf *btf, int type_id)
> +{
> +	static char buf[16 * 1024];
> +	FILE *buf_file;
> +
> +	buf_file = fmemopen(buf, sizeof(buf) - 1, "w");
> +	if (!buf_file) {
> +		fprintf(stderr, "Failed to open memstream: %d\n", errno);
> +		return NULL;
> +	}
> +
> +	fprintf_btf_type_raw(buf_file, btf, type_id);
> +	fflush(buf_file);
> +	fclose(buf_file);
> +
> +	return buf;
> +}
> diff --git a/tools/testing/selftests/bpf/btf_helpers.h b/tools/testing/selftests/bpf/btf_helpers.h
> new file mode 100644
> index 000000000000..2c9ce1b61dc9
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/btf_helpers.h
> @@ -0,0 +1,12 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (c) 2020 Facebook */
> +#ifndef __BTF_HELPERS_H
> +#define __BTF_HELPERS_H
> +
> +#include <stdio.h>
> +#include <bpf/btf.h>
> +
> +int fprintf_btf_type_raw(FILE *out, const struct btf *btf, __u32 id);
> +const char *btf_type_raw_dump(const struct btf *btf, int type_id);
> +
> +#endif
> diff --git a/tools/testing/selftests/bpf/prog_tests/btf_write.c b/tools/testing/selftests/bpf/prog_tests/btf_write.c
> index 314e1e7c36df..bc1412de1b3d 100644
> --- a/tools/testing/selftests/bpf/prog_tests/btf_write.c
> +++ b/tools/testing/selftests/bpf/prog_tests/btf_write.c
> @@ -2,6 +2,7 @@
> /* Copyright (c) 2020 Facebook */
> #include <test_progs.h>
> #include <bpf/btf.h>
> +#include "btf_helpers.h"
> 
> static int duration = 0;
> 
> @@ -11,12 +12,12 @@ void test_btf_write() {
> 	const struct btf_member *m;
> 	const struct btf_enum *v;
> 	const struct btf_param *p;
> -	struct btf *btf;
> +	struct btf *btf = NULL;

No need to initialize btf. 

> 	int id, err, str_off;
> 
> 	btf = btf__new_empty();
> 	if (CHECK(IS_ERR(btf), "new_empty", "failed: %ld\n", PTR_ERR(btf)))
> -		return;
> +		goto err_out;

err_out is not needed either. 

[...]