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 <[email protected]> > --- > 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'
