Prev: tracing update v2
Next: [PATCH] [ARM] .gitignore: Ignore piggy.lzo, lib1funcs.S and vmlinux
From: Jean Delvare on 23 Apr 2010 05:40
Hi Farid, Wolfram,

On Fri, 23 Apr 2010 00:01:55 +0200, Farid Hammane wrote:
> This patch fixes coding style issues found by checkpatch.pl.
> i2c-algo-pca.c has been built successfully after applying this patch.
>
> Signed-off-by: Farid Hammane <farid.hammane(a)gmail.com>
> ---
> drivers/i2c/algos/i2c-algo-pca.c | 36 ++++++++++++++++++------------------
> 1 files changed, 18 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/i2c/algos/i2c-algo-pca.c b/drivers/i2c/algos/i2c-algo-pca.c
> index dcdaf8e..ca817f8 100644
> --- a/drivers/i2c/algos/i2c-algo-pca.c
> +++ b/drivers/i2c/algos/i2c-algo-pca.c
> @@ -37,15 +37,15 @@
>
> static int i2c_debug;
>
> -#define pca_outw(adap, reg, val) adap->write_byte(adap->data, reg, val)
> -#define pca_inw(adap, reg) adap->read_byte(adap->data, reg)
> +#define pca_outw(adap, reg, val) (adap->write_byte(adap->data, reg, val))
> +#define pca_inw(adap, reg) (adap->read_byte(adap->data, reg))

I'm confused by these changes. For one thing, macros which are
shortcuts for function calls normally don't need surrounding
parentheses. If checkpatch.pl complains about that, I would call it a
false positive, unless someone can prove me wrong with a real-world
case where these surrounding parentheses help.

For another, macro _parameters_ normally need parentheses for each
non-trivial use. I would thus expect the following to be correct:

#define pca_outw(adap, reg, val) (adap)->write_byte((adap)->data, reg, val)
#define pca_inw(adap, reg) (adap)->read_byte((adap)->data, reg)

Or is it just me?

>
> #define pca_status(adap) pca_inw(adap, I2C_PCA_STA)
> -#define pca_clock(adap) adap->i2c_clock
> +#define pca_clock(adap) (adap->i2c_clock)
> #define pca_set_con(adap, val) pca_outw(adap, I2C_PCA_CON, val)
> #define pca_get_con(adap) pca_inw(adap, I2C_PCA_CON)
> -#define pca_wait(adap) adap->wait_for_completion(adap->data)
> -#define pca_reset(adap) adap->reset_chip(adap->data)
> +#define pca_wait(adap) (adap->wait_for_completion(adap->data))
> +#define pca_reset(adap) (adap->reset_chip(adap->data))

Same here...

I'm fine with all other changes. But checkpatch.pl spouts 2 more errors
to me, which we've discussed before. I'm curious why you didn't fix
them? Just replace each block of 8 spaces with one tab.

ERROR: code indent should use tabs where possible
#181: FILE: i2c/algos/i2c-algo-pca.c:181:
+ struct i2c_msg *msgs,$

ERROR: code indent should use tabs where possible
#182: FILE: i2c/algos/i2c-algo-pca.c:182:
+ int num)$

--
Jean Delvare
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Jean Delvare on 25 Apr 2010 08:10
Hi Farid,

On Sat, 24 Apr 2010 16:34:14 +0200, Farid Hammane wrote:
> This patch fixes up some coding style issues. i2c-algo-pca.c has been
> build successfully after applying this patch and the binary object is
> still exactly the same.
> Other issues found by checkpatch.pl were voluntarily not fixed, either
> to keep readability, or because of false positive errors.
>
> Signed-off-by: Farid Hammane <farid.hammane(a)gmail.com>
> ---
> drivers/i2c/algos/i2c-algo-pca.c | 36 ++++++++++++++++++------------------
> 1 files changed, 18 insertions(+), 18 deletions(-)

Applied, thanks.

--
Jean Delvare
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 | 
Pages: 1
Prev: tracing update v2
Next: [PATCH] [ARM] .gitignore: Ignore piggy.lzo, lib1funcs.S and vmlinux