Messages in this thread Patch in this message | | | From | "Verma, Vishal L" <> | Subject | Re: [PATCH 11/13] nvdimm: Use more common logic testing styles and bare ; positions | Date | Thu, 12 Sep 2019 03:52:49 +0000 |
| |
On Wed, 2019-09-11 at 19:54 -0700, Joe Perches wrote: > Avoid using uncommon logic testing styles to make the code a > bit more like other kernel code. > > e.g.: > if (foo) { > ; > } else { > <code> > } > > is typically written > > if (!foo) { > <code> > } >
A lot of times the excessive inversions seem to result in a net loss of readability - e.g.:
<snip>
> diff --git a/drivers/nvdimm/region_devs.c > b/drivers/nvdimm/region_devs.c > index 65df07481909..6861e0997d21 100644 > --- a/drivers/nvdimm/region_devs.c > +++ b/drivers/nvdimm/region_devs.c > @@ -320,9 +320,7 @@ static ssize_t set_cookie_show(struct device *dev, > struct nd_interleave_set *nd_set = nd_region->nd_set; > ssize_t rc = 0; > > - if (is_memory(dev) && nd_set) > - /* pass, should be precluded by region_visible */;
For one, the comment is lost
> - else > + if (!(is_memory(dev) && nd_set))
And it takes a moment to resolve between things such as:
if (!(A && B)) vs. if (!(A) && B)
And this is especially true if 'A' and 'B' are longer function calls, split over multiple lines, or are themselves compound 'sections'.
I'm not opposed to /all/ such transformations -- for example, the ones where the logical inversion can be 'consumed' by toggling a comparision operator, as you have a few times in this patch, don't sacrifice any readibility, and perhaps even improve it.
> return -ENXIO; > > /*
| |