On Mon, 09 Nov 2020 13:44:48 -0800 John Fastabend <john.fastabend@xxxxxxxxx> wrote: > Alex Shi wrote: > > > > > > 在 2020/11/7 上午12:13, Jesper Dangaard Brouer 写道: > > > Hmm... REG_STATE_NEW is zero, so it is implicitly set via memset zero. > > > But it is true that it is technically not directly used or referenced. > > > > > > It is mentioned in a comment, so please send V2 with this additional change: > > > > Hi Jesper, > > > > Thanks a lot for comments. here is the v2: > > > > From 2908d25bf2e1c90ad71a83ba056743f45da283e8 Mon Sep 17 00:00:00 2001 > > From: Alex Shi <alex.shi@xxxxxxxxxxxxxxxxx> > > Date: Fri, 6 Nov 2020 13:41:58 +0800 > > Subject: [PATCH v2] net/xdp: remove unused macro REG_STATE_NEW > > > > To tame gcc warning on it: > > net/core/xdp.c:20:0: warning: macro "REG_STATE_NEW" is not used > > [-Wunused-macros] > > And change related comments as Jesper Dangaard Brouer suggested. > > > > Signed-off-by: Alex Shi <alex.shi@xxxxxxxxxxxxxxxxx> > > --- > > > net/core/xdp.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/net/core/xdp.c b/net/core/xdp.c > > index 48aba933a5a8..0df5ee5682d9 100644 > > --- a/net/core/xdp.c > > +++ b/net/core/xdp.c > > @@ -19,7 +19,6 @@ > > #include <trace/events/xdp.h> > > #include <net/xdp_sock_drv.h> > > > > -#define REG_STATE_NEW 0x0 > > #define REG_STATE_REGISTERED 0x1 > > #define REG_STATE_UNREGISTERED 0x2 > > #define REG_STATE_UNUSED 0x3 > > I think having the define there makes it more readable and clear what > the zero state is. But if we run with unused-macros I guess its even > uglier to try and mark it with unused attribute. I agree having the define there makes it more readable and clear what the zero state is. We can also add code that replace the comment, that check/use these defines. It is slow-path code, so it doesn't hurt to add this extra code. Generally I find it strange to "fix" these kind of warnings, but also don't care that we do fix them if it helps someone else spot code where it actually matters. > Acked-by: John Fastabend <john.fastabend@xxxxxxxxx> > > > @@ -175,7 +174,7 @@ int xdp_rxq_info_reg(struct xdp_rxq_info *xdp_rxq, > > return -ENODEV; > > } > > > > - /* State either UNREGISTERED or NEW */ > > + /* State either UNREGISTERED or zero */ > > xdp_rxq_info_init(xdp_rxq); > > xdp_rxq->dev = dev; > > xdp_rxq->queue_index = queue_index; > > -- > > 1.8.3.1 > > > > -- Best regards, Jesper Dangaard Brouer MSc.CS, Principal Kernel Engineer at Red Hat LinkedIn: http://www.linkedin.com/in/brouer
