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; > > /*