Re: [PATCH net] ethernet: xircom: small clean up in setup_xirc2ps_cs()
From: Dan Carpenter Date: Mon, 21 Aug 2017 12:47:30 +0300 > The get_options() function takes the whole ARRAY_SIZE(). It doesn't > matter here because we don't use more than 7 elements. > > Signed-off-by: Dan Carpenter Applied, thanks.
Re: [PATCH net] ethernet: xircom: small clean up in setup_xirc2ps_cs()
On Mon, Aug 21, 2017 at 02:52:34PM +, David Laight wrote: > From: Dan Carpenter > > Sent: 21 August 2017 10:48 > > The get_options() function takes the whole ARRAY_SIZE(). It doesn't > > matter here because we don't use more than 7 elements. > > > > Signed-off-by: Dan Carpenter > > > > diff --git a/drivers/net/ethernet/xircom/xirc2ps_cs.c > > b/drivers/net/ethernet/xircom/xirc2ps_cs.c > > index f71883264cc0..fd5288ff53b5 100644 > > --- a/drivers/net/ethernet/xircom/xirc2ps_cs.c > > +++ b/drivers/net/ethernet/xircom/xirc2ps_cs.c > > @@ -1781,7 +1781,7 @@ static int __init setup_xirc2ps_cs(char *str) > > */ > > int ints[10] = { -1 }; > > > > - str = get_options(str, 9, ints); > > + str = get_options(str, ARRAY_SIZE(ints), ints); > > > > #define MAYBE_SET(X,Y) if (ints[0] >= Y && ints[Y] != -1) { X = ints[Y]; } > > MAYBE_SET(if_port, 3); > > That code looks very dubious to me. > It looks as though it expects all of the ints[] array to be initialised > to -1, not just the first element. I'm pretty sure you're right and I thought about changing it when I sent the patch but it doesn't matter. It's not required to initialize that code anyway because either get_options() will initialize it or it won't be used. regards, dan carpenter
RE: [PATCH net] ethernet: xircom: small clean up in setup_xirc2ps_cs()
From: Dan Carpenter > Sent: 21 August 2017 10:48 > The get_options() function takes the whole ARRAY_SIZE(). It doesn't > matter here because we don't use more than 7 elements. > > Signed-off-by: Dan Carpenter > > diff --git a/drivers/net/ethernet/xircom/xirc2ps_cs.c > b/drivers/net/ethernet/xircom/xirc2ps_cs.c > index f71883264cc0..fd5288ff53b5 100644 > --- a/drivers/net/ethernet/xircom/xirc2ps_cs.c > +++ b/drivers/net/ethernet/xircom/xirc2ps_cs.c > @@ -1781,7 +1781,7 @@ static int __init setup_xirc2ps_cs(char *str) >*/ > int ints[10] = { -1 }; > > - str = get_options(str, 9, ints); > + str = get_options(str, ARRAY_SIZE(ints), ints); > > #define MAYBE_SET(X,Y) if (ints[0] >= Y && ints[Y] != -1) { X = ints[Y]; } > MAYBE_SET(if_port, 3); That code looks very dubious to me. It looks as though it expects all of the ints[] array to be initialised to -1, not just the first element. David
[PATCH net] ethernet: xircom: small clean up in setup_xirc2ps_cs()
The get_options() function takes the whole ARRAY_SIZE(). It doesn't matter here because we don't use more than 7 elements. Signed-off-by: Dan Carpenter diff --git a/drivers/net/ethernet/xircom/xirc2ps_cs.c b/drivers/net/ethernet/xircom/xirc2ps_cs.c index f71883264cc0..fd5288ff53b5 100644 --- a/drivers/net/ethernet/xircom/xirc2ps_cs.c +++ b/drivers/net/ethernet/xircom/xirc2ps_cs.c @@ -1781,7 +1781,7 @@ static int __init setup_xirc2ps_cs(char *str) */ int ints[10] = { -1 }; - str = get_options(str, 9, ints); + str = get_options(str, ARRAY_SIZE(ints), ints); #define MAYBE_SET(X,Y) if (ints[0] >= Y && ints[Y] != -1) { X = ints[Y]; } MAYBE_SET(if_port, 3);