lkml.org 
[lkml]   [2017]   [Sep]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] Staging: iio: adc: Added Space around binary op.
From
Date
On 09/08/2017 11:32 AM, Jonathan Cameron wrote:
> On Fri, 8 Sep 2017 07:29:06 +0100
> Jonathan Cameron <jic23@jic23.retrosnub.co.uk> wrote:
>
>> On 8 September 2017 05:47:52 BST, Himanshi Jain <himshijain.hj@gmail.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@gmail.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.

> Jonathan
>
>>
>> Jonathan
>>
>>
>>> ---
>>> drivers/staging/iio/adc/ad7192.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/staging/iio/adc/ad7192.c
>>> b/drivers/staging/iio/adc/ad7192.c
>>> index d11c6de..1aee662 100644
>>> --- a/drivers/staging/iio/adc/ad7192.c
>>> +++ b/drivers/staging/iio/adc/ad7192.c
>>> @@ -341,7 +341,7 @@ static int ad7192_setup(struct ad7192_state *st,
>>> }
>>>
>>> static IIO_DEVICE_ATTR_NAMED(in_v_m_v_scale_available,
>>> - in_voltage-voltage_scale_available,
>>> + in_voltage - voltage_scale_available,
>>> 0444, ad7192_show_scale_available, NULL, 0);
>>>
>>> static IIO_DEVICE_ATTR(in_voltage_scale_available, 0444,
>>
>

\
 
 \ /
  Last update: 2017-09-08 11:39    [W:0.436 / U:1.236 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site