Re: [PATCH v2 2/8] staging: vt6655: Use incrementation in `idx`

[Dan Carpenter]

On Thu, Nov 04, 2021 at 07:44:22AM -0700, Joe Perches wrote:
> On Thu, 2021-11-04 at 16:00 +0300, Dan Carpenter wrote:
> > On Mon, Nov 01, 2021 at 02:32:00PM +0000, Karolina Drobnik wrote:
> > > Increment `idx` in a loop instead of adding the loop counter
> > > `i` to do so. Thanks to this change, the cast to unsigned short
> > > can be removed.
> > > 
> > > Signed-off-by: Karolina Drobnik <karolinadrobnik@xxxxxxxxx>
> > > ---
> > >  drivers/staging/vt6655/rf.c | 16 ++++++++--------
> > >  1 file changed, 8 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/drivers/staging/vt6655/rf.c b/drivers/staging/vt6655/rf.c
> []
> > > @@ -700,11 +700,11 @@ bool RFvWriteWakeProgSyn(struct vnt_private *priv, unsigned char rf_type,
> > >  			return false;
> > >  
> > >  		for (i = 0; i < CB_AL2230_INIT_SEQ; i++)
> > > -			MACvSetMISCFifo(priv, (unsigned short)(idx + i), al2230_init_table[i]);
> > > +			MACvSetMISCFifo(priv, idx++, al2230_init_table[i]);
> > 
> > Of course, idx is no longer a constant so declaring it as a variable
> > makes sense here.  But maybe just do it in the same patch because the
> > patch 1/1 doesn't make sense as a stand alone patch.
> > 
> > Also don't declare idx as an unsigned short.  It's better to declare it
> > as a int so it just works like a normal number and you don't have to
> > think about signedness bugs and wrapping and edge cases.
> 
> No, IMO it really should be a u16.
> Look at the iowrite16() within the call to MACvSetMISCFifo.

Obviously we all agree it has no effect on runtime at all.

I just think that people get into trouble getting too creative with
their types.  There was some discussion earlier about where the casting
was required (it's not required at all).  It seems like there was some
confusion about how types work.  Declaring it as "unsigned short" has
some documentation value, I guess, but you avoid signedness bugs when
you declare things as int.

I don't care too much, but I also try to tell people not to be fancy.

regards,
dan carpenter