Re: [PATCH v3 bpf-next 1/3] bpf: switch BPF UAPI #define constants used from BPF program side to enums

[Michael Forney <mforney@mforney.org>]

On 2020-06-02, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> ISO C forbids zero-size arrays, unnamed struct/union, gcc extensions,
> empty unions, etc
> So ?

So their use should be avoided in UAPI headers whenever possible.
While the other extensions are simple and have clear semantics, enums
with out-of-range values do not. Even gcc and clang don't agree on
what the types of out-of-range constants are within the enum
specifier:

	enum { A = 0x80000000ULL, is_clang =
__builtin_types_compatible_p(__typeof__(A), unsigned long long) };

On gcc x86_64, is_clang == 0. On clang x86_64, is_clang == 1.

> #define BPF_F_CTXLEN_MASK BPF_F_CTXLEN_MASK
> will only work as workaround for _your_ compiler.
> We are not gonna add hacks like this for every compiler.

This doesn't solve the problem, which is that after preprocessing,
BPF_F_INDEX_MASK and BPF_F_CURRENT_CPU, and BPF_F_CTXLEN_MASK are enum
constants with values that can't be represented as int. Changing them
back to defines will. This is not a hack, it is following the C
standard.

> and I still don't see how it breaks anything.
> If it was #define and .h switched its definition from 1 to 1ULL
> it would have had the same effect. That is the point of the constant in .h.
> Same effect regardless whether it was done via #define and via enum.
> The size and type of the constant may change. It's not uapi breakage.

My point is that something like

	unsigned long long x = BPF_DEVCG_DEV_BLOCK << 32;

used to be perfectly fine in Linux 5.6. Now, in 5.7 with the enum
definition, it is undefined behavior.

-Michael