From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754443AbdIHJqn (ORCPT ); Fri, 8 Sep 2017 05:46:43 -0400 Received: from szxga05-in.huawei.com ([45.249.212.191]:5577 "EHLO szxga05-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753292AbdIHJql (ORCPT ); Fri, 8 Sep 2017 05:46:41 -0400 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> Organization: Huawei X-Mailer: Claws Mail 3.15.0 (GTK+ 2.24.31; x86_64-w64-mingw32) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.206.48.115] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A090201.59B266FB.00B3,ss=1,re=0.000,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0, ip=0.0.0.0, so=2014-11-16 11:51:01, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: caaef987b64bbca50d84e04b2b5143ba Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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, > >> > > >