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

From	Julia Lawall <julia.lawall@lip6.fr>
Newsgroups	linux.kernel
Subject	Re: [Outreachy kernel] Re: [PATCH] Staging: iio: adc: Added Space around binary op.
Date	2017-09-08 12:40 +0200
Message-ID	<unoRz-5F0-3@gated-at.bofh.it> (permalink)
References	(1 earlier) <unkXE-2YQ-7@gated-at.bofh.it> <unnVw-4Ss-11@gated-at.bofh.it> <unnVw-4Ss-9@gated-at.bofh.it> <unooy-5ko-11@gated-at.bofh.it> <unoye-5qa-9@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw


On Fri, 8 Sep 2017, Lars-Peter Clausen wrote:

> 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@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.
> >
> > 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.

OK, it looks like a good project :)

julia

Thread

[PATCH] Staging: iio: adc: Added Space around binary op. Himanshi Jain <himshijain.hj@gmail.com> - 2017-09-08 06:50 +0200
  Re: [Outreachy kernel] [PATCH] Staging: iio: adc: Added Space around  binary op. Julia Lawall <julia.lawall@lip6.fr> - 2017-09-08 08:10 +0200
  Re: [PATCH] Staging: iio: adc: Added Space around binary op. Jonathan Cameron <jic23@jic23.retrosnub.co.uk> - 2017-09-08 08:30 +0200
    Re: [PATCH] Staging: iio: adc: Added Space around binary op. Lars-Peter Clausen <lars@metafoo.de> - 2017-09-08 11:40 +0200
      Re: [PATCH] Staging: iio: adc: Added Space around binary op. Jonathan Cameron <Jonathan.Cameron@huawei.com> - 2017-09-08 11:50 +0200
      Re: [Outreachy kernel] Re: [PATCH] Staging: iio: adc: Added Space  around binary op. Julia Lawall <julia.lawall@lip6.fr> - 2017-09-08 12:10 +0200
        Re: [Outreachy kernel] Re: [PATCH] Staging: iio: adc: Added Space  around binary op. Lars-Peter Clausen <lars@metafoo.de> - 2017-09-08 12:20 +0200
          Re: [Outreachy kernel] Re: [PATCH] Staging: iio: adc: Added Space  around binary op. Julia Lawall <julia.lawall@lip6.fr> - 2017-09-08 12:40 +0200
    Re: [PATCH] Staging: iio: adc: Added Space around binary op. Jonathan Cameron <Jonathan.Cameron@huawei.com> - 2017-09-08 11:40 +0200

csiph-web