lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Thu, 22 Apr 2010 09:25:24 +0200
From:	Jean Delvare <khali@...ux-fr.org>
To:	Wolfram Sang <w.sang@...gutronix.de>
Cc:	Farid Hammane <farid.hammane@...il.com>, ben-linux@...ff.org,
	Enrik.Berkhan@...com, linux-i2c@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] i2c-algo-pca: fix all coding style issues in 
 i2c-algo-pca.c

On Thu, 22 Apr 2010 01:57:38 +0200, Wolfram Sang wrote:
> Hi Farid,
> 
> thanks for this approach. Have you checked that the binary is the same
> before/after your patch? If so, please mention in your patch description.
> 
> Also, always keep in mind that checkpatch helps to make code readable. Some of
> your changes should keep readability in mind not just fixing the warnings.
> 
> On Wed, Apr 21, 2010 at 09:11:37PM +0200, Farid Hammane wrote:
> > This patch fixes all coding style issues found by checkpatch.pl.
> 
> > 
> > Signed-off-by: Farid Hammane <farid.hammane@...il.com>
> > ---
> >  drivers/i2c/algos/i2c-algo-pca.c |   74 ++++++++++++++++++++++---------------
> >  1 files changed, 44 insertions(+), 30 deletions(-)
> > 
> > diff --git a/drivers/i2c/algos/i2c-algo-pca.c b/drivers/i2c/algos/i2c-algo-pca.c
> > (...)
> > @@ -178,12 +178,12 @@ static int pca_rx_ack(struct i2c_algo_pca_data *adap,
> >  }
> >  
> >  static int pca_xfer(struct i2c_adapter *i2c_adap,
> > -                    struct i2c_msg *msgs,
> > -                    int num)
> > +		struct i2c_msg *msgs,
> > +		int num)
> 
> One more tab maybe?

Better use tab + spaces and align on the opening parenthesis. What
checkpatch.pl complains about here isn't the alignment, it's the use of
more than 8 consecutive spaces.

> > (...)
> > @@ -241,8 +244,10 @@ static int pca_xfer(struct i2c_adapter *i2c_adap,
> >  			completed = pca_address(adap, msg);
> >  			break;
> >  
> > -		case 0x18: /* SLA+W has been transmitted; ACK has been received */
> > -		case 0x28: /* Data byte in I2CDAT has been transmitted; ACK has been received */
> > +		case 0x18: /* SLA+W has been transmitted;
> > +			      ACK has been received */
> > +		case 0x28: /* Data byte in I2CDAT has been transmitted;
> > +			      ACK has been received */
> 
> First, check CodingStyle for how multiline comments should look like. For
> readability, I'd like to keep them single line, though. I think this could
> be done by rewording. Same for all following comments.

Please keep in mind that Farid doesn't know the code. He is "only"
helping with formatting cleanups. Asking him to reword the comments
doesn't seem wise. And there's nothing wrong with two-line comments as
above. The "preferred comment format" in CodingStyle is often
unrealistic IMHO, I'm not always following it in my own code.

I agree with all other comments.

-- 
Jean Delvare
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists