Re: [PATCH v2 1/4] staging: pi433: Style fix - Correct long lines

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi everybody,

from my point of view, we should stay with the old implementation.

Ok - line is too long according to style guide. But these long lines are 
IMHO easy to read:
All four are pretty similar. By having all Tokens in exact the same length 
and having one below other, you can easily detect the differences between
the lines and that's important. As soon as you start to wrap them 
- regardless how - you won't be able to detect the differences that easy 
any more - and from my Point of view that's a disadvantage.

Cheers,

Marcus

> Dan Carpenter <dan.carpenter@xxxxxxxxxx> hat am 22. August 2017 um 12:03
> geschrieben:
>
>
> On Tue, Aug 22, 2017 at 02:57:49PM +0530, Rishabh Hardas wrote:
> > On Sat, Aug 19, 2017 at 09:47:28PM -0700, Joe Perches wrote:
> > > On Wed, 2017-08-16 at 10:31 +0300, Dan Carpenter wrote:
> > > > On Wed, Aug 16, 2017 at 10:53:18AM +0530, Rishabh Hardas wrote:
> > > > > @@ -143,10 +142,13 @@ struct pi433_rx_cfg {
> > > > >
> > > > > #define PI433_IOC_MAGIC 'r'
> > > > >
> > > > > -#define PI433_IOC_RD_TX_CFG _IOR(PI433_IOC_MAGIC,
> > > > > PI433_TX_CFG_IOCTL_NR, char[sizeof(struct pi433_tx_cfg)])
> > > > > -#define PI433_IOC_WR_TX_CFG _IOW(PI433_IOC_MAGIC,
> > > > > PI433_TX_CFG_IOCTL_NR, char[sizeof(struct pi433_tx_cfg)])
> > > > > -
> > > > > -#define PI433_IOC_RD_RX_CFG _IOR(PI433_IOC_MAGIC,
> > > > > PI433_RX_CFG_IOCTL_NR, char[sizeof(struct pi433_rx_cfg)])
> > > > > -#define PI433_IOC_WR_RX_CFG _IOW(PI433_IOC_MAGIC,
> > > > > PI433_RX_CFG_IOCTL_NR, char[sizeof(struct pi433_rx_cfg)])
> > > > > +#define PI433_IOC_RD_TX_CFG _IOR(PI433_IOC_MAGIC,
> > > > > PI433_TX_CFG_IOCTL_NR,\
> > > > > + char[sizeof(struct pi433_tx_cfg)])
> > > > > +#define PI433_IOC_WR_TX_CFG _IOW(PI433_IOC_MAGIC,
> > > > > PI433_TX_CFG_IOCTL_NR,\
> > > > > + char[sizeof(struct pi433_tx_cfg)])
> > > > > +#define PI433_IOC_RD_RX_CFG _IOR(PI433_IOC_MAGIC,
> > > > > PI433_RX_CFG_IOCTL_NR,\
> > > > > + char[sizeof(struct pi433_rx_cfg)])
> > > > > +#define PI433_IOC_WR_RX_CFG _IOW(PI433_IOC_MAGIC,
> > > > > PI433_RX_CFG_IOCTL_NR,\
> > > > > + char[sizeof(struct pi433_rx_cfg)])
> > > >
> > > >
> > > > These don't help readability. The original was better.
> > >
> > > The original wasn't any good either.
> > >
> > > It'd be better to avoid the macros altogether
> > > as almost all are use-once.
> > >
> > >
> > So should I keep this as it is or remove the macros ?
> > Because as Dan said the corrections that I made aren't goo either.
> >
>
> Find a way to correct it which makes the code more readable than it was
> before.
>
> regards,
> dan carpenter
>
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux