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

[Dan Carpenter]

I'm skeptical that these are the absolute platonic ideal of what the
code should look like...  You may be right that it's mathematically
impossible to make these lines more readable, but we can't really debate
alternate versions of this until someone sends us a patch.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

[Marcus Wolf]

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  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
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

[Dan Carpenter]

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
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

[Rishabh Hardas]

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.

Regards
Rishabh Hardas
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

[Joe Perches]

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.


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

[Dan Carpenter]

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.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

[Greg KH]

On Thu, Aug 03, 2017 at 05:20:43PM +0530, Rishabh Hardas wrote:
> Signed-off-by: Rishabh Hardas 
> ---

I can not take patches without any changelog text at all, sorry.

Please fix up and resend this series.

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel