Re: [PATCH bpf-next v1 19/19] selftests/bpf: Fix errors compiling cg_storage_multi.h with musl libc

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jul 23, 2024 at 03:35:22PM -0700, YiFei Zhu wrote:
> On Mon, Jul 22, 2024 at 10:56 PM Tony Ambardar <tony.ambardar@xxxxxxxxx> wrote:
> >
> > Remove a redundant include of '<asm/types.h>', whose needed definitions are
> > already included (via '<linux/types.h>') in cg_storage_multi_egress_only.c,
> > cg_storage_multi_isolated.c, and cg_storage_multi_shared.c. This avoids
> > redefinition errors seen compiling for mips64el/musl-libc like:
> >
> >   In file included from progs/cg_storage_multi_egress_only.c:13:
> >   In file included from progs/cg_storage_multi.h:6:
> >   In file included from /usr/mips64el-linux-gnuabi64/include/asm/types.h:23:
> >   /usr/include/asm-generic/int-l64.h:29:25: error: typedef redefinition with different types ('long' vs 'long long')
> >      29 | typedef __signed__ long __s64;
> >         |                         ^
> >   /usr/include/asm-generic/int-ll64.h:30:44: note: previous definition is here
> >      30 | __extension__ typedef __signed__ long long __s64;
> >         |                                            ^
> >
> > Fixes: 9e5bd1f7633b ("selftests/bpf: Test CGROUP_STORAGE map can't be used by multiple progs")
> > Signed-off-by: Tony Ambardar <tony.ambardar@xxxxxxxxx>
> > ---
> >  tools/testing/selftests/bpf/progs/cg_storage_multi.h | 2 --
> >  1 file changed, 2 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/progs/cg_storage_multi.h b/tools/testing/selftests/bpf/progs/cg_storage_multi.h
> > index a0778fe7857a..41d59f0ee606 100644
> > --- a/tools/testing/selftests/bpf/progs/cg_storage_multi.h
> > +++ b/tools/testing/selftests/bpf/progs/cg_storage_multi.h
> > @@ -3,8 +3,6 @@
> >  #ifndef __PROGS_CG_STORAGE_MULTI_H
> >  #define __PROGS_CG_STORAGE_MULTI_H
> >
> > -#include <asm/types.h>
> > -
> >  struct cgroup_value {
> >         __u32 egress_pkts;
> >         __u32 ingress_pkts;
> > --
> > 2.34.1
> >
> 
> Hmm, some linter checks prefer headers themselves include everything
> they use. This header uses __u32 and after this patch it would include
> no headers. Would it be okay to include <linux/types.h> or we don't
> care?
> 
> YiFei Zhu

Good point, I agree even the readability suffers without headers.
Replacing <asm/types.h> with <linux/types.h> makes more sense, and
guards in the latter avoid the conflicts noted above. I'll queue this
change for a v2. Appreciate the feedback!

Cheers,
Tony Ambardar




[Index of Archives]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux