From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753899AbdIHJiT (ORCPT ); Fri, 8 Sep 2017 05:38:19 -0400 Received: from www381.your-server.de ([78.46.137.84]:52827 "EHLO www381.your-server.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751654AbdIHJiR (ORCPT ); Fri, 8 Sep 2017 05:38:17 -0400 Subject: Re: [PATCH] Staging: iio: adc: Added Space around binary op. To: Jonathan Cameron , Jonathan Cameron Cc: Himanshi Jain , outreachy-kernel@googlegroups.com, Michael.Hennerich@analog.com, jic23@kernel.org, knaack.h@gmx.de, pmeerw@pmeerw.net, gregkh@linuxfoundation.org, linux-iio@vger.kernel.org, devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org References: <20170908044752.GA6199@himanshi-Inspiron-5558> <20170908103257.00002165@huawei.com> From: Lars-Peter Clausen Message-ID: <2e3476f0-9e7e-2701-220a-5fb178d68d2e@metafoo.de> Date: Fri, 8 Sep 2017 11:37:56 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <20170908103257.00002165@huawei.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Authenticated-Sender: lars@metafoo.de Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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. > 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, >> >