Re: [PATCH 07/31] e1000: Use more constant definitions

[BALATON Zoltan]

On Thu, 12 Jan 2023, Philippe Mathieu-Daudé wrote:
> On 12/1/23 10:57, Akihiko Odaki wrote:
> > The definitions for E1000_VFTA_ENTRY_SHIFT, E1000_VFTA_ENTRY_MASK, and
> > E1000_VFTA_ENTRY_BIT_SHIFT_MASK were copied from:
> > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/net/ethernet/intel/e1000/e1000_hw.h?h=v6.0.9#n306
> >
> > The definitions for E1000_NUM_UNICAST, E1000_MC_TBL_SIZE, and
> > E1000_VLAN_FILTER_TBL_SIZE were copied from:
> > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/net/ethernet/intel/e1000/e1000_hw.h?h=v6.0.9#n707
> >
> > Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> > ---
> >   hw/net/e1000.c         | 50 +++++++++++++++++++++++-------------------
> >   hw/net/e1000_regs.h    |  9 ++++++++
> >   hw/net/e1000x_common.c |  5 +++--
> >   hw/net/e1000x_common.h |  2 +-
> >   4 files changed, 41 insertions(+), 25 deletions(-)
>
>
> > diff --git a/hw/net/e1000x_common.h b/hw/net/e1000x_common.h
> > index 3501e4855a..b991d814b1 100644
> > --- a/hw/net/e1000x_common.h
> > +++ b/hw/net/e1000x_common.h
> > @@ -102,7 +102,7 @@ enum {
> >   static inline void
> >   e1000x_inc_reg_if_not_full(uint32_t *mac, int index)
> >   {
> > -    if (mac[index] != 0xffffffff) {
> > +    if (mac[index] != UINT32_MAX) {
>
> I wonder if using -1 wouldn't be simpler, otherwise great

Comparing unsigned type with negative value does not seem very clean to 
me. Either of the above (0xffffffff or UINT32_MAX) is more explicit and 
easier to understand.

Regards,
BALATON Zoltan

> cleanup!
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
> >           mac[index]++;
> >       }
> >   }