linux-kernel - Re: [Outreachy kernel] Re: [PATCH] Staging: iio: adc: Added Space around binary op.

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:   Fri, 8 Sep 2017 12:11:56 +0200
From:   Lars-Peter Clausen <lars@...afoo.de>
To:     Julia Lawall <julia.lawall@...6.fr>
Cc:     Jonathan Cameron <Jonathan.Cameron@...wei.com>,
        Jonathan Cameron <jic23@...23.retrosnub.co.uk>,
        Himanshi Jain <himshijain.hj@...il.com>,
        outreachy-kernel@...glegroups.com, Michael.Hennerich@...log.com,
        jic23@...nel.org, knaack.h@....de, pmeerw@...erw.net,
        gregkh@...uxfoundation.org, linux-iio@...r.kernel.org,
        devel@...verdev.osuosl.org, linux-kernel@...r.kernel.org
Subject: Re: [Outreachy kernel] Re: [PATCH] Staging: iio: adc: Added Space
 around binary op.

On 09/08/2017 11:59 AM, Julia Lawall wrote:
> 
> 
> On Fri, 8 Sep 2017, Lars-Peter Clausen wrote:
> 
>> On 09/08/2017 11:32 AM, Jonathan Cameron wrote:
>>> On Fri, 8 Sep 2017 07:29:06 +0100
>>> Jonathan Cameron <jic23@...23.retrosnub.co.uk> wrote:
>>>
>>>> On 8 September 2017 05:47:52 BST, Himanshi Jain <himshijain.hj@...il.com> wrote:
>>>>> Added space around(one on each side of) binary
>>>>> operator(-) as preferred according to kernel
>>>>> coding style.
>>>>>
>>>>> Signed-off-by: Himanshi Jain <himshijain.hj@...il.com>
>>>>
>>>> Take a closer look at that macro. It isn't doing what you think...
>>>> To give a hint, changing this breaks userspace.
>>>
>>> Ok, I'm bored of this particular one coming up. When you have
>>> worked out what is going on Himanshi, would you mind putting
>>> together a patch adding a comment describing why it is a bad
>>> idea to 'fix' this?  That would be a very useful patch as
>>> far as I'm concerned :)
>>>
>>> There aren't that many cases of this in IIO so adding a comment
>>> on each of them is probably reasonable just to avoid wasting
>>> people's time on fixing them! (I think we have had more than
>>> 5 such goes this year so far...)
>>>
>>
>> I'd much rather fix this API so that you have to put "" around the name so
>> that it is clear that it is a string, rather than doing the implicit
>> conversion to string using preprocessor magic. Better to have a
>> self-documenting API then having to add a comment to each user of the API.
> 
> All the DEVICE_ATTR macros use the same strategy.  And the non-IIO ones,
> eg DEVICE_ATTR_RW, also use the provided name to construct the names of
> the show and store functions, so I don't think a string would be
> appropriate.

I'm only suggesting to use a string for the _NAMED macros where the name is
not used to construct the identifiers.

In the case where the name is used to construct the identifiers we don't
have the issue with false positives since the name must be a valid
identifier on its own.