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]

Date:   Thu, 28 Oct 2021 15:31:55 +0100
From:   Karolina Drobnik <karolinadrobnik@...il.com>
To:     "Fabio M. De Francesco" <fmdefrancesco@...il.com>,
        Julia Lawall <julia.lawall@...ia.fr>
Cc:     outreachy-kernel@...glegroups.com, gregkh@...uxfoundation.org,
        forest@...ttletooquiet.net, linux-staging@...ts.linux.dev,
        linux-kernel@...r.kernel.org
Subject: Re: [Outreachy kernel] [PATCH 4/7] staging: vt6655: Introduce
 `data` temporary variable

Hi Fabio and Julia,

Thank you very much for looking at my changes.

On Thu, 2021-10-28 at 13:21 +0200, Fabio M. De Francesco wrote:
> 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).

Yes, I added them on purpose to improve readability (+ that's also my
preference anyway)

> 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 understand, thanks for letting me know.


On Thu, 2021-10-28 at 13:32 +0200, Julia Lawall wrote:
> Would it be better as data[i] ?
> 
> Could there be a better name than "data"?  Perhaps "table"?

Hmm, now when I'm thinking about, it indeed looks like a better option.
I would even say that `data` (or `table`/`init_table`) can be only used
in the AL7320 case and we can go with `al2230_init_table` for AL2230.
The line would be 61 characters long, way below the limit.

What do you think?


Many thanks,
Karolina