linux-kernel - Re: [Outreachy kernel] [PATCH 4/7] staging: vt6655: Introduce `data` temporary variable

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.22.394.2110281332040.9518@hadrien>
Date:   Thu, 28 Oct 2021 13:32:58 +0200 (CEST)
From:   Julia Lawall <julia.lawall@...ia.fr>
To:     "Fabio M. De Francesco" <fmdefrancesco@...il.com>
cc:     outreachy-kernel@...glegroups.com, gregkh@...uxfoundation.org,
        forest@...ttletooquiet.net, linux-staging@...ts.linux.dev,
        linux-kernel@...r.kernel.org,
        Karolina Drobnik <karolinadrobnik@...il.com>,
        Karolina Drobnik <karolinadrobnik@...il.com>
Subject: Re: [Outreachy kernel] [PATCH 4/7] staging: vt6655: Introduce `data`
 temporary variable



On Thu, 28 Oct 2021, Fabio M. De Francesco wrote:

> On Thursday, October 28, 2021 12:35:34 PM CEST Karolina Drobnik wrote:
> > Add a variable to store initialization tables. Use this variable
> > in AL2230 initialization.
> >
> > Signed-off-by: Karolina Drobnik <karolinadrobnik@...il.com>
> > ---
> >  drivers/staging/vt6655/rf.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/vt6655/rf.c b/drivers/staging/vt6655/rf.c
> > index ea74701917e5..afd202ea3356 100644
> > --- a/drivers/staging/vt6655/rf.c
> > +++ b/drivers/staging/vt6655/rf.c
> > @@ -684,6 +684,7 @@ bool RFvWriteWakeProgSyn(struct vnt_private *priv,
> unsigned char rf_type,
> >  	unsigned short idx = MISCFIFO_SYNDATA_IDX;
> >  	unsigned char init_count = 0;
> >  	unsigned char sleep_count = 0;
> > +	const unsigned long *data;
> >
> >  	VNSvOutPortW(iobase + MAC_REG_MISCFFNDEX, 0);
> >  	switch (rf_type) {
> > @@ -699,8 +700,9 @@ bool RFvWriteWakeProgSyn(struct vnt_private *priv,
> unsigned char rf_type,
> >  		if (init_count > (MISCFIFO_SYNDATASIZE - sleep_count))
> >  			return false;
> >
> > +		data = al2230_init_table;
> >  		for (i = 0; i < CB_AL2230_INIT_SEQ; i++)
> > -			MACvSetMISCFifo(priv, idx++,
> al2230_init_table[i]);
> > +			MACvSetMISCFifo(priv, idx++, *(data++));
>
> Hi Karolina,
>
> I think you are using redundant parentheses in "* (data ++)" but understand
> that those increments and dereferences are equivalent to "* data ++"
> (according to the C precedence rules).
>
> Some time ago I suggested that you use those redundant parentheses because
> Greg Kroah-Hartman had previously explained that he prefers not to see
> "*foo++" because maintainers and reviewers are not required to remember the C
> precedence rules.
>
> I hope my suggestion isn't based on a misunderstanding of what Greg wants
> here and that your patch can be accepted as is.
>
> While we are at it, please notice that Maintainers of different subsystems
> may see this topic from a different point of view and that they might very
> well ask you for removing those redundant parentheses.
>
> I'd suggest to use grep and find out what is preferred in other subsystems,
> when you want to contribute to other parts of Linux.

Would it be better as data[i] ?

Could there be a better name than "data"?  Perhaps "table"?

julia


>
> Thanks,
>
> Fabio
>
> >  		MACvSetMISCFifo(priv, idx++,
> al2230_channel_table0[channel - 1]);
> >  		MACvSetMISCFifo(priv, idx++,
> al2230_channel_table1[channel - 1]);
> > --
> > 2.30.2
> >
> > --
> > You received this message because you are subscribed to the Google Groups
> "outreachy-kernel" group.
> > To unsubscribe from this group and stop receiving emails from it, send an
> email to outreachy-kernel+unsubscribe@...glegroups.com.
> > To view this discussion on the web visit https://groups.google.com/d/msgid/
> outreachy-kernel/dc72a4c3539aed70569f66396ed3b51818bc2aea.
> 1635415820.git.karolinadrobnik%40gmail.com.
> >
>
>
>
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@...glegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/2039159.k92FijXA2m%40localhost.localdomain.
>