On Mon, 11 Dec 2017, Joe Perches wrote:
> > - I don't understand the error for xa_head here:
> >
> > struct xarray {
> > spinlock_t xa_lock;
> > gfp_t xa_flags;
> > void __rcu * xa_head;
> > };
> >
> > Do people really think that:
> >
> > struct xarray {
> > spinlock_t xa_lock;
> > gfp_t xa_flags;
> > void __rcu *xa_head;
> > };
> >
> > is more aesthetically pleasing? And not just that, but it's an *error*
> > so the former is *RIGHT* and this is *WRONG*. And not just a matter
Not sure what was meant here. Neither one is *WRONG* in the sense of
being invalid C code. In the sense of what checkpatch will accept, the
former is *WRONG* and the latter is *RIGHT* -- the opposite of what
was written.
> > of taste?
>
> No opinion really.
> That's from Andy Whitcroft's original implementation.
This one, at least, is easy to explain. The original version tends to
lead to bugs, or easily misunderstood code. Consider if another
variable was added to the declaration; it could easily turn into:
void __rcu * xa_head, xa_head2;
(The compiler will reject this, but it wouldn't if the underlying type
had been int instead of void.)
Doing it the other way makes the meaning a lot more clear:
void __rcu *xa_head, *xa_head2;
This is an idiom specifically intended to reduce the likelihood of
errors. Rather like avoiding assignments inside conditionals.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
