On Tue, Mar 26, 2019 at 06:05:24PM +0000, Jiong Wang wrote:
> "enum bpf_reg_liveness" is actually used as bit instead of integer. For
> example:
>
> if (live & (REG_LIVE_READ | REG_LIVE_WRITTEN | REG_LIVE_DONE))
>
> Using enum to represent bit is error prone, better to use explicit bit
> macros.
>
> Signed-off-by: Jiong Wang <jiong.wang@xxxxxxxxxxxxx>
> ---
> include/linux/bpf_verifier.h | 16 +++++++++-------
> kernel/bpf/verifier.c | 5 ++---
> 2 files changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
> index 7d8228d..f03c86a 100644
> --- a/include/linux/bpf_verifier.h
> +++ b/include/linux/bpf_verifier.h
> @@ -34,12 +34,14 @@
> * but of the link between it and its parent. See mark_reg_read() and
> * mark_stack_slot_read() in kernel/bpf/verifier.c.
> */
> -enum bpf_reg_liveness {
> - REG_LIVE_NONE = 0, /* reg hasn't been read or written this branch */
> - REG_LIVE_READ, /* reg was read, so we're sensitive to initial value */
> - REG_LIVE_WRITTEN, /* reg was written first, screening off later reads */
> - REG_LIVE_DONE = 4, /* liveness won't be updating this register anymore */
> -};
yes. it is enum that is used as a bitfield, but I prefer to keep it as enum
because clang -Wassign-enum can do at least some type checking.
I also find it easier to review the code when it has
'enum bpf_reg_liveness' instead of 'u8'
