Re: [Outreachy kernel] [PATCH] staging: vt6655: rearrange lines exceeding 100 columns

[Deepak R Varma <mh12gx2825@gmail.com>]

On Wed, Oct 14, 2020 at 06:46:09PM +0200, Julia Lawall wrote:
> 
> 
> On Wed, 14 Oct 2020, Deepak R Varma wrote:
> 
> > On Wed, Oct 14, 2020 at 06:01:39PM +0200, Julia Lawall wrote:
> > >
> > >
> > > On Wed, 14 Oct 2020, Deepak R Varma wrote:
> > >
> > > > Rearrange lines that are longer than 100 columns width. Issue reported
> > > > by chckpatch script.
> > > >
> > > > Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
> > > >
> > > > ---
> > > >
> > > >  drivers/staging/vt6655/device_main.c | 8 ++++++--
> > > >  1 file changed, 6 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/staging/vt6655/device_main.c b/drivers/staging/vt6655/device_main.c
> > > > index 09ab6d6f2429..4fc90b111f9d 100644
> > > > --- a/drivers/staging/vt6655/device_main.c
> > > > +++ b/drivers/staging/vt6655/device_main.c
> > > > @@ -461,7 +461,10 @@ static bool device_init_rings(struct vnt_private *priv)
> > > >  		priv->opts.rx_descs0 * sizeof(struct vnt_rx_desc);
> > > >
> > > >  	priv->tx0_bufs = dma_alloc_coherent(&priv->pcid->dev,
> > > > -					    priv->opts.tx_descs[0] * PKT_BUF_SZ + priv->opts.tx_descs[1] * PKT_BUF_SZ + CB_BEACON_BUF_SIZE + CB_MAX_BUF_SIZE,
> > > > +					    priv->opts.tx_descs[0] * PKT_BUF_SZ +
> > > > +					    priv->opts.tx_descs[1] * PKT_BUF_SZ +
> > > > +					    CB_BEACON_BUF_SIZE +
> > > > +					    CB_MAX_BUF_SIZE,
> > > >  					    &priv->tx_bufs_dma0, GFP_ATOMIC);
> > > >  	if (!priv->tx0_bufs) {
> > > >  		dev_err(&priv->pcid->dev, "allocate buf dma memory failed\n");
> > > > @@ -1077,7 +1080,8 @@ static void vnt_interrupt_process(struct vnt_private *priv)
> > > >  			    priv->op_mode == NL80211_IFTYPE_ADHOC) &&
> > > >  			    priv->vif->bss_conf.enable_beacon) {
> > > >  				MACvOneShotTimer1MicroSec(priv,
> > > > -							  (priv->vif->bss_conf.beacon_int - MAKE_BEACON_RESERVED) << 10);
> > > > +							  (priv->vif->bss_conf.beacon_int -
> > > > +							   MAKE_BEACON_RESERVED) << 10);
> > >
> > > Maybe it would make more sense to shorten the function name?
> > >
> > Thank you Julia.
> > I do not think that will work. Even without the function name,
> > the line would still cross 100 columns if you try to fit it on a single
> > line. Also, shortening the function name would make it looks different
> > from rest of similar named functions.
> > Please suggest how would you do it?
> 
> Perhaps the whole thing can't fit on one line, but if the function name
> were shorter, then perhaps the newline would go in a different place.
> 
> All of the function names in that file do not follow the kernel coding
> style.  Admittedly it would not be such a great idea to change only one of
> them.
> 
> > > Also the {} are not needed.
> > >
> > Agree if we can get the instruction on a single line. Else, I think it
> > is better to keep the {} to make it more readable.
> 
> Coding style doesn't say anything about that.  The only exception proposed
> is that when one of the branches requires {}, then the other should use {}
> as well.
> 
> linux/Documentation/process/coding-style.rst
> 
Yes, agreed. I will implement this feedback and resend v2.

Thank you again!
Deepak.

> julia