From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Fri, 8 Sep 2017 10:46:17 +0100 From: Jonathan Cameron To: Lars-Peter Clausen CC: Jonathan Cameron , Himanshi Jain , , , , , , , , , Subject: Re: [PATCH] Staging: iio: adc: Added Space around binary op. Message-ID: <20170908104617.00001edf@huawei.com> In-Reply-To: <2e3476f0-9e7e-2701-220a-5fb178d68d2e@metafoo.de> References: <20170908044752.GA6199@himanshi-Inspiron-5558> <20170908103257.00002165@huawei.com> <2e3476f0-9e7e-2701-220a-5fb178d68d2e@metafoo.de> MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" List-ID: On Fri, 8 Sep 2017 11:37:56 +0200 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 wrote: > > > >> On 8 September 2017 05:47:52 BST, Himanshi Jain wrote: > >>> Added space around(one on each side of) binary > >>> operator(-) as preferred according to kernel > >>> coding style. > >>> > >>> Signed-off-by: Himanshi Jain > >> > >> 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. Good point. So Himanshi, feel like taking this on? Jonathan > > > 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, > >> > > >