linux-kernel - Re: [PATCH] i2c-algo-pca: fix all coding style issues in i2c-algo-pca.c

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]

Message-ID: <20100422092524.77b7aa84@hyperion.delvare>
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/