Re: [PATCH bpf-next] selftests/bpf: move struct definitions out of function params
From: Andrii Nakryiko
Date: Thu Dec 22 2022 - 14:04:12 EST
On Tue, Dec 20, 2022 at 9:59 PM James Hilliard
<james.hilliard1@xxxxxxxxx> wrote:
>
> Anonymous structs can't be declared inside function parameter
> definitions in current c standards, however clang doesn't detect this
> condition currently while GCC does.
>
> Details: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108189
>
> Fixes errors like:
> progs/btf_dump_test_case_bitfields.c:85:7: error: anonymous struct declared inside parameter list will not be visible outside of this definition or declaration [-Werror]
> 85 | int f(struct {
> | ^~~~~~
>
> Signed-off-by: James Hilliard <james.hilliard1@xxxxxxxxx>
> ---
> .../bpf/progs/btf_dump_test_case_bitfields.c | 9 ++++--
> .../progs/btf_dump_test_case_namespacing.c | 10 ++++---
> .../bpf/progs/btf_dump_test_case_packing.c | 10 ++++---
> .../bpf/progs/btf_dump_test_case_padding.c | 10 ++++---
> .../bpf/progs/btf_dump_test_case_syntax.c | 30 +++++++++++++------
> 5 files changed, 46 insertions(+), 23 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/progs/btf_dump_test_case_bitfields.c b/tools/testing/selftests/bpf/progs/btf_dump_test_case_bitfields.c
> index e01690618e1e..c75f6bd06a49 100644
> --- a/tools/testing/selftests/bpf/progs/btf_dump_test_case_bitfields.c
> +++ b/tools/testing/selftests/bpf/progs/btf_dump_test_case_bitfields.c
> @@ -82,11 +82,16 @@ struct bitfield_flushed {
> long b: 16;
> };
>
> -int f(struct {
> +/* ----- START-EXPECTED-OUTPUT ----- */
> +struct root_struct {
there is no need to make this struct part of expected output, just
keep it next to f?
> struct bitfields_only_mixed_types _1;
> struct bitfield_mixed_with_others _2;
> struct bitfield_flushed _3;
> -} *_)
> +};
> +
> +/* ------ END-EXPECTED-OUTPUT ------ */
> +
> +int f(struct root_struct *_)
> {
> return 0;
> }
> diff --git a/tools/testing/selftests/bpf/progs/btf_dump_test_case_namespacing.c b/tools/testing/selftests/bpf/progs/btf_dump_test_case_namespacing.c
> index 92a4ad428710..d7cf2a8487c9 100644
> --- a/tools/testing/selftests/bpf/progs/btf_dump_test_case_namespacing.c
> +++ b/tools/testing/selftests/bpf/progs/btf_dump_test_case_namespacing.c
> @@ -49,9 +49,7 @@ typedef int Y;
>
> typedef int Z;
>
> -/*------ END-EXPECTED-OUTPUT ------ */
> -
> -int f(struct {
> +struct root_struct {
> struct S _1;
> S _2;
> union U _3;
> @@ -67,7 +65,11 @@ int f(struct {
> X xx;
> Y yy;
> Z zz;
> -} *_)
> +};
same, that struct is only to preserve all the referenced types, so
keep it hidden from the output
> +
> +/*------ END-EXPECTED-OUTPUT ------ */
> +
> +int f(struct root_struct *_)
> {
> return 0;
> }
> diff --git a/tools/testing/selftests/bpf/progs/btf_dump_test_case_packing.c b/tools/testing/selftests/bpf/progs/btf_dump_test_case_packing.c
> index 7998f27df7dd..e039ceb50c43 100644
> --- a/tools/testing/selftests/bpf/progs/btf_dump_test_case_packing.c
> +++ b/tools/testing/selftests/bpf/progs/btf_dump_test_case_packing.c
> @@ -132,9 +132,7 @@ struct outer_packed_struct {
> struct nested_packed_struct b;
> } __attribute__((packed));
>
> -/* ------ END-EXPECTED-OUTPUT ------ */
> -
> -int f(struct {
> +struct root_struct {
> struct packed_trailing_space _1;
> struct non_packed_trailing_space _2;
> struct packed_fields _3;
> @@ -147,7 +145,11 @@ int f(struct {
> struct usb_host_endpoint _10;
> struct outer_nonpacked_struct _11;
> struct outer_packed_struct _12;
> -} *_)
> +};
> +
> +/* ------ END-EXPECTED-OUTPUT ------ */
> +
> +int f(struct root_struct *_)
> {
> return 0;
> }
> diff --git a/tools/testing/selftests/bpf/progs/btf_dump_test_case_padding.c b/tools/testing/selftests/bpf/progs/btf_dump_test_case_padding.c
> index 79276fbe454a..2ca46ad8d66a 100644
> --- a/tools/testing/selftests/bpf/progs/btf_dump_test_case_padding.c
> +++ b/tools/testing/selftests/bpf/progs/btf_dump_test_case_padding.c
> @@ -220,9 +220,7 @@ struct outer_mixed_but_unpacked {
> struct nested_packed b2;
> };
>
> -/* ------ END-EXPECTED-OUTPUT ------ */
> -
> -int f(struct {
> +struct root_struct {
> struct padded_implicitly _1;
> struct padded_explicitly _2;
> struct padded_a_lot _3;
> @@ -243,7 +241,11 @@ int f(struct {
> struct ib_wc _201;
> struct acpi_object_method _202;
> struct outer_mixed_but_unpacked _203;
> -} *_)
> +} __attribute__((packed));
> +
> +/* ------ END-EXPECTED-OUTPUT ------ */
> +
> +int f(struct root_struct *_)
> {
> return 0;
> }
> diff --git a/tools/testing/selftests/bpf/progs/btf_dump_test_case_syntax.c b/tools/testing/selftests/bpf/progs/btf_dump_test_case_syntax.c
> index 26fffb02ed10..3e31df7cecc6 100644
> --- a/tools/testing/selftests/bpf/progs/btf_dump_test_case_syntax.c
> +++ b/tools/testing/selftests/bpf/progs/btf_dump_test_case_syntax.c
> @@ -104,24 +104,24 @@ typedef void (*printf_fn_t)(const char *, ...);
> * typedef const fn_output_inner_t fn_ptr_arr2_t[5];
> */
> /* ----- START-EXPECTED-OUTPUT ----- */
> -typedef char * const * (*fn_ptr2_t)(struct {
> - int a;
> -}, int (*)(int));
> +struct struct_a;
> +
> +typedef char * const * (*fn_ptr2_t)(struct struct_a, int (*)(int));
> +
> +struct struct_c;
> +
> +struct struct_h;
>
> typedef struct {
> int a;
> - void (*b)(int, struct {
> - int c;
> - }, union {
> + void (*b)(int, struct struct_c, union {
> char d;
> int e[5];
> });
> } (*fn_complex_t)(union {
> void *f;
> char g[16];
> -}, struct {
> - int h;
> -});
> +}, struct struct_h);
these do test some pieces of libbpf's btf_dump logic, so I'm way more
reluctant to remove these. If I understand correctly, this syntax will
be eventually supported by GCC, so is there any way to keep these
examples as is by requiring C23 mode or something? Or just skipping
compiling this one if GCC is used?
>
> typedef void (* (*signal_t)(int, void (*)(int)))(int);
>
> @@ -272,6 +272,18 @@ struct root_struct {
> struct float_struct _15;
> };
>
> +struct struct_a {
> + int a;
> +};
> +
> +struct struct_h {
> + int h;
> +};
> +
> +struct struct_c {
> + int c;
> +};
> +
> /* ------ END-EXPECTED-OUTPUT ------ */
>
> int f(struct root_struct *s)
> --
> 2.34.1
>