linux-i2c.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/25] treewide-next: Use static const char arrays
@ 2010-09-13 19:47 Joe Perches
       [not found] ` <cover.1284406638.git.joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>
  2010-09-14  9:14 ` (unknown) David Howells
  0 siblings, 2 replies; 10+ messages in thread
From: Joe Perches @ 2010-09-13 19:47 UTC (permalink / raw)
  To: linux-kernel
  Cc: Ralf Baechle, Benjamin Herrenschmidt, Paul Mackerras, Len Brown,
	Linus Walleij, Jean Delvare (PC drivers, core),
	Ben Dooks (embedded platforms), Karsten Keil,
	Mauro Carvalho Chehab, Jeff Kirsher, Jesse Brandeburg,
	Bruce Allan, Alex Duyck, PJ Waskiewicz, John Ronciak,
	Amit Kumar Salecha, Anirban Chakraborty, linux-driver

Using static const char foo[] = "bar" can save some
code and text space, so change the places where it's possible.

Also change the places that use
	char foo[] = "barX";
	...
	foo[3] = value + '0';
where X is typically changed
	char foo[sizeof("barX")];
	...
	sprintf(foo, "bar%c", value + '0');

Joe Perches (25):
  arch/mips: Use static const char arrays
  arch/powerpc: Use static const char arrays
  drivers/acpi: Use static const char arrays
  drivers/char: Use static const char arrays
  drivers/i2c: Use static const char arrays
  drivers/isdn: Use static const char arrays
  drivers/media: Use static const char arrays
  drivers/net/atl1c: Use static const char arrays
  drivers/net/atl1e: Use static const char arrays
  drivers/net/(intel): Use static const char arrays
  drivers/net/netxen: Use static const char arrays
  drivers/net/qlcnic: Use static const char arrays
  drivers/net/spider_net.c: Use static const char arrays
  drivers/net/vnxnet3: Use static const char arrays
  drivers/net/wireless/ipw2x00: Use static const char arrays
  drivers/s390/char: Use static const char arrays
  drivers/scsi: Use static const char arrays
  drivers/serial/suncore.c: Use static const char arrays
  drivers/staging: Use static const char arrays
  drivers/usb: Use static const char arrays
  drivers/video: Use static const char arrays
  net/dsa: Use static const char arrays
  net/sunrpc: Use static const char arrays
  sound: Use static const char arrays
  tools/perf/util: Use static const char arrays

 arch/mips/pnx8550/common/reset.c                   |    4 ++--
 arch/powerpc/boot/addnote.c                        |    4 ++--
 arch/powerpc/boot/cuboot-c2k.c                     |    4 ++--
 arch/powerpc/kernel/irq.c                          |    2 +-
 drivers/acpi/sleep.c                               |    4 ++--
 drivers/char/hvc_vio.c                             |    2 +-
 drivers/i2c/busses/i2c-stu300.c                    |    4 ++--
 drivers/isdn/hysdn/hycapi.c                        |    2 +-
 drivers/isdn/mISDN/dsp_cmx.c                       |    2 +-
 drivers/media/video/zoran/zoran_device.c           |    5 ++---
 drivers/net/atl1c/atl1c.h                          |    4 ++--
 drivers/net/atl1c/atl1c_main.c                     |    4 ++--
 drivers/net/atl1e/atl1e.h                          |    4 ++--
 drivers/net/atl1e/atl1e_main.c                     |    4 ++--
 drivers/net/e1000/e1000.h                          |    2 +-
 drivers/net/e1000/e1000_main.c                     |    4 ++--
 drivers/net/e1000e/e1000.h                         |    2 +-
 drivers/net/e1000e/netdev.c                        |    2 +-
 drivers/net/igb/igb.h                              |    4 ++--
 drivers/net/igb/igb_main.c                         |    4 ++--
 drivers/net/igbvf/igbvf.h                          |    2 +-
 drivers/net/igbvf/netdev.c                         |    2 +-
 drivers/net/ixgb/ixgb.h                            |    2 +-
 drivers/net/ixgb/ixgb_main.c                       |    2 +-
 drivers/net/ixgbe/ixgbe.h                          |    2 +-
 drivers/net/ixgbe/ixgbe_main.c                     |    4 ++--
 drivers/net/ixgbevf/ixgbevf.h                      |    2 +-
 drivers/net/ixgbevf/ixgbevf_main.c                 |    2 +-
 drivers/net/netxen/netxen_nic.h                    |    2 +-
 drivers/net/netxen/netxen_nic_main.c               |    2 +-
 drivers/net/qlcnic/qlcnic.h                        |    2 +-
 drivers/net/qlcnic/qlcnic_main.c                   |    2 +-
 drivers/net/spider_net.c                           |    2 +-
 drivers/net/vmxnet3/vmxnet3_drv.c                  |    2 +-
 drivers/net/vmxnet3/vmxnet3_int.h                  |    2 +-
 drivers/net/wireless/ipw2x00/ipw2100.c             |    2 +-
 drivers/net/wireless/ipw2x00/ipw2200.c             |    2 +-
 drivers/net/wireless/ipw2x00/libipw_module.c       |    2 +-
 drivers/s390/char/vmlogrdr.c                       |    4 ++--
 drivers/scsi/bnx2i/bnx2i_hwi.c                     |    6 +++---
 drivers/scsi/lpfc/lpfc_init.c                      |    2 +-
 drivers/scsi/megaraid/megaraid_mbox.c              |    6 +++---
 drivers/serial/suncore.c                           |    4 ++--
 drivers/staging/brcm80211/util/bcmutils.c          |    2 +-
 drivers/staging/comedi/drivers/comedi_bond.c       |    2 +-
 drivers/staging/cxt1e1/ossiRelease.c               |    2 +-
 drivers/staging/go7007/go7007-driver.c             |    2 +-
 drivers/staging/msm/mdp.c                          |    2 +-
 .../staging/rtl8192e/ieee80211/ieee80211_module.c  |    2 +-
 .../staging/rtl8192u/ieee80211/ieee80211_module.c  |    2 +-
 drivers/staging/tidspbridge/rmgr/dbdcd.c           |    6 +++---
 drivers/usb/atm/ueagle-atm.c                       |   14 +++++---------
 drivers/usb/otg/langwell_otg.c                     |    2 +-
 drivers/video/sh_mipi_dsi.c                        |    4 ++--
 drivers/video/sis/sis_main.c                       |   10 +++++-----
 drivers/video/via/viafbdev.c                       |    2 +-
 net/dsa/dsa.c                                      |    2 +-
 net/dsa/dsa_priv.h                                 |    2 +-
 net/sunrpc/auth_gss/gss_krb5_mech.c                |    2 +-
 sound/core/misc.c                                  |    5 ++++-
 tools/perf/util/ui/setup.c                         |    3 ++-
 tools/perf/util/ui/util.c                          |    3 ++-
 62 files changed, 98 insertions(+), 98 deletions(-)

-- 
1.7.3.rc1

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 05/25] drivers/i2c: Use static const char arrays
       [not found] ` <cover.1284406638.git.joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>
@ 2010-09-13 19:47   ` Joe Perches
       [not found]     ` <b0c83cb1b42c6adf38f6322066fb457693f7ac3b.1284406638.git.joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Joe Perches @ 2010-09-13 19:47 UTC (permalink / raw)
  To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: Linus Walleij, Jean Delvare (PC drivers, core),
	Ben Dooks (embedded platforms),
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA

Signed-off-by: Joe Perches <joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>
---
 drivers/i2c/busses/i2c-stu300.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-stu300.c b/drivers/i2c/busses/i2c-stu300.c
index 495be45..2f7c09c 100644
--- a/drivers/i2c/busses/i2c-stu300.c
+++ b/drivers/i2c/busses/i2c-stu300.c
@@ -871,7 +871,7 @@ stu300_probe(struct platform_device *pdev)
 	struct resource *res;
 	int bus_nr;
 	int ret = 0;
-	char clk_name[] = "I2C0";
+	char clk_name[sizeof("I2Cx")];
 
 	dev = kzalloc(sizeof(struct stu300_dev), GFP_KERNEL);
 	if (!dev) {
@@ -881,7 +881,7 @@ stu300_probe(struct platform_device *pdev)
 	}
 
 	bus_nr = pdev->id;
-	clk_name[3] += (char)bus_nr;
+	sprintf(clk_name, "I2C%c", '0' + bus_nr);
 	dev->clk = clk_get(&pdev->dev, clk_name);
 	if (IS_ERR(dev->clk)) {
 		ret = PTR_ERR(dev->clk);
-- 
1.7.3.rc1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 05/25] drivers/i2c: Use static const char arrays
       [not found]     ` <b0c83cb1b42c6adf38f6322066fb457693f7ac3b.1284406638.git.joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>
@ 2010-09-13 19:56       ` Russell King - ARM Linux
  2010-09-13 20:08       ` Jonathan Cameron
  2010-09-14 23:23       ` Ben Dooks
  2 siblings, 0 replies; 10+ messages in thread
From: Russell King - ARM Linux @ 2010-09-13 19:56 UTC (permalink / raw)
  To: Joe Perches
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	Jean Delvare (PC drivers, core), linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	Linus Walleij, Ben Dooks (embedded platforms),
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On Mon, Sep 13, 2010 at 12:47:43PM -0700, Joe Perches wrote:
> Signed-off-by: Joe Perches <joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>
> ---
>  drivers/i2c/busses/i2c-stu300.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-stu300.c b/drivers/i2c/busses/i2c-stu300.c
> index 495be45..2f7c09c 100644
> --- a/drivers/i2c/busses/i2c-stu300.c
> +++ b/drivers/i2c/busses/i2c-stu300.c
> @@ -871,7 +871,7 @@ stu300_probe(struct platform_device *pdev)
>  	struct resource *res;
>  	int bus_nr;
>  	int ret = 0;
> -	char clk_name[] = "I2C0";
> +	char clk_name[sizeof("I2Cx")];
>  
>  	dev = kzalloc(sizeof(struct stu300_dev), GFP_KERNEL);
>  	if (!dev) {
> @@ -881,7 +881,7 @@ stu300_probe(struct platform_device *pdev)
>  	}
>  
>  	bus_nr = pdev->id;
> -	clk_name[3] += (char)bus_nr;
> +	sprintf(clk_name, "I2C%c", '0' + bus_nr);

If people would implement the clk API correctly (matching by struct device
rather than giving every clock a name and trying to use the const char *
as the sole matching token), stuff like this would not be necessary.
The second argument to clk_get() is supposed to be a _consumer_ name
(iow, something to determine which clock for the struct device you want)
rather than a producer name.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 05/25] drivers/i2c: Use static const char arrays
       [not found]     ` <b0c83cb1b42c6adf38f6322066fb457693f7ac3b.1284406638.git.joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>
  2010-09-13 19:56       ` Russell King - ARM Linux
@ 2010-09-13 20:08       ` Jonathan Cameron
       [not found]         ` <4C8E84A0.1010606-tko9wxEg+fIOOJlXag/Snyp2UmYkHbXO@public.gmane.org>
  2010-09-14  7:36         ` Lothar Waßmann
  2010-09-14 23:23       ` Ben Dooks
  2 siblings, 2 replies; 10+ messages in thread
From: Jonathan Cameron @ 2010-09-13 20:08 UTC (permalink / raw)
  To: Joe Perches
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	Jean Delvare (PC drivers, core), linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	Linus Walleij, Ben Dooks (embedded platforms),
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

 Commit message is  somewhat inaccurate...

On 09/13/10 20:47, Joe Perches wrote:
> Signed-off-by: Joe Perches <joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>
> ---
>  drivers/i2c/busses/i2c-stu300.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-stu300.c b/drivers/i2c/busses/i2c-stu300.c
> index 495be45..2f7c09c 100644
> --- a/drivers/i2c/busses/i2c-stu300.c
> +++ b/drivers/i2c/busses/i2c-stu300.c
> @@ -871,7 +871,7 @@ stu300_probe(struct platform_device *pdev)
>  	struct resource *res;
>  	int bus_nr;
>  	int ret = 0;
> -	char clk_name[] = "I2C0";
> +	char clk_name[sizeof("I2Cx")];
>  
>  	dev = kzalloc(sizeof(struct stu300_dev), GFP_KERNEL);
>  	if (!dev) {
> @@ -881,7 +881,7 @@ stu300_probe(struct platform_device *pdev)
>  	}
>  
>  	bus_nr = pdev->id;
> -	clk_name[3] += (char)bus_nr;
> +	sprintf(clk_name, "I2C%c", '0' + bus_nr);
I'm guessing that there are never more than a couple of these.
Why is this method a better bet than just putting %d?
>  	dev->clk = clk_get(&pdev->dev, clk_name);
>  	if (IS_ERR(dev->clk)) {
>  		ret = PTR_ERR(dev->clk);

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 05/25] drivers/i2c: Use static const char arrays
       [not found]         ` <4C8E84A0.1010606-tko9wxEg+fIOOJlXag/Snyp2UmYkHbXO@public.gmane.org>
@ 2010-09-13 20:16           ` Joe Perches
  0 siblings, 0 replies; 10+ messages in thread
From: Joe Perches @ 2010-09-13 20:16 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	Jean Delvare (PC drivers, core), linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	Linus Walleij, Ben Dooks (embedded platforms),
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On Mon, 2010-09-13 at 21:08 +0100, Jonathan Cameron wrote:
> Commit message is  somewhat inaccurate...

Yeah, sorry 'bout that.
That's what I get for using a script.
I did write an intro with more complete description.

> > diff --git a/drivers/i2c/busses/i2c-stu300.c b/drivers/i2c/busses/i2c-stu300.c
> > @@ -881,7 +881,7 @@ stu300_probe(struct platform_device *pdev)
> >  	}
> >  
> >  	bus_nr = pdev->id;
> > -	clk_name[3] += (char)bus_nr;
> > +	sprintf(clk_name, "I2C%c", '0' + bus_nr);
> I'm guessing that there are never more than a couple of these.
> Why is this method a better bet than just putting %d?

It tries to standardize the style use and it avoids possible
future checkpatch warnings of:
	char foo[] = "bar"
char array could possibly be static const.

There was another use with "%1.1d" somewhere.

The end result is the same, so I don't really care much
if this sort of change is applied or not.  The possible
checkpatch message could just be considered noise but
Mike Frysinger seemed to prefer it, so I thought I could
try to accommodate him.

cheers, Joe

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 05/25] drivers/i2c: Use static const char arrays
  2010-09-13 20:08       ` Jonathan Cameron
       [not found]         ` <4C8E84A0.1010606-tko9wxEg+fIOOJlXag/Snyp2UmYkHbXO@public.gmane.org>
@ 2010-09-14  7:36         ` Lothar Waßmann
       [not found]           ` <19599.9719.562406.692216-VjFSrY7JcPWvSplVBqRQBQ@public.gmane.org>
  1 sibling, 1 reply; 10+ messages in thread
From: Lothar Waßmann @ 2010-09-14  7:36 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Linus Walleij, linux-kernel, Jean Delvare (PC drivers, core),
	linux-i2c, Ben Dooks (embedded platforms), Joe Perches,
	linux-arm-kernel

Hi,

Jonathan Cameron writes:
>  Commit message is  somewhat inaccurate...
> 
> On 09/13/10 20:47, Joe Perches wrote:
> > Signed-off-by: Joe Perches <joe@perches.com>
> > ---
> >  drivers/i2c/busses/i2c-stu300.c |    4 ++--
> >  1 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-stu300.c b/drivers/i2c/busses/i2c-stu300.c
> > index 495be45..2f7c09c 100644
> > --- a/drivers/i2c/busses/i2c-stu300.c
> > +++ b/drivers/i2c/busses/i2c-stu300.c
> > @@ -871,7 +871,7 @@ stu300_probe(struct platform_device *pdev)
> >  	struct resource *res;
> >  	int bus_nr;
> >  	int ret = 0;
> > -	char clk_name[] = "I2C0";
> > +	char clk_name[sizeof("I2Cx")];
> >  
> >  	dev = kzalloc(sizeof(struct stu300_dev), GFP_KERNEL);
> >  	if (!dev) {
> > @@ -881,7 +881,7 @@ stu300_probe(struct platform_device *pdev)
> >  	}
> >  
> >  	bus_nr = pdev->id;
> > -	clk_name[3] += (char)bus_nr;
> > +	sprintf(clk_name, "I2C%c", '0' + bus_nr);
> I'm guessing that there are never more than a couple of these.
> Why is this method a better bet than just putting %d?
>
'%c' will only ever produce one byte of output while '%d' may
produce up to 11 bytes depending on the value of bus_nr thus
overflowing the buffer.


Lothar Waßmann
-- 
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info@karo-electronics.de
___________________________________________________________

^ permalink raw reply	[flat|nested] 10+ messages in thread

* (unknown)
  2010-09-13 19:47 [PATCH 00/25] treewide-next: Use static const char arrays Joe Perches
       [not found] ` <cover.1284406638.git.joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>
@ 2010-09-14  9:14 ` David Howells
  1 sibling, 0 replies; 10+ messages in thread
From: David Howells @ 2010-09-14  9:14 UTC (permalink / raw)
  To: Joe Perches
  Cc: Amit Kumar Salecha, linux-fbdev, linux-usb, Karsten Keil,
	James Smart, linux-mips, VMware, Inc., Bruce Allan, PJ Waskiewicz,
	Shreyas Bhatewara, alsa-devel, Jaroslav Kysela, dhowells,
	James E.J. Bottomley, Paul Mackerras, linux-i2c, Brett Rudley,
	sparclinux, devel, linux-s390, linux-scsi,
	Florian Tobias Schandinat, e1000-devel, Jesse Brandeburg,
	linux-acpi

Joe Perches <joe@perches.com> wrote:

> Using static const char foo[] = "bar" can save some
> code and text space, so change the places where it's possible.

That's reasonable.

> Also change the places that use
> 	char foo[] = "barX";
> 	...
> 	foo[3] = value + '0';
> where X is typically changed
> 	char foo[sizeof("barX")];
> 	...
> 	sprintf(foo, "bar%c", value + '0');

You haven't said what this gains.  I can see what it may cost, though
(depending on how gcc loads foo[]).

David

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 05/25] drivers/i2c: Use static const char arrays
       [not found]           ` <19599.9719.562406.692216-VjFSrY7JcPWvSplVBqRQBQ@public.gmane.org>
@ 2010-09-14 12:57             ` Jonathan Cameron
       [not found]               ` <4C8F7157.9060005-tko9wxEg+fIOOJlXag/Snyp2UmYkHbXO@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Jonathan Cameron @ 2010-09-14 12:57 UTC (permalink / raw)
  To: Lothar Waßmann
  Cc: Jonathan Cameron, Joe Perches, Linus Walleij,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA, Ben Dooks (embedded platforms),
	Jean Delvare (PC drivers, core),
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On 09/14/10 08:36, Lothar Waßmann wrote:
> Hi,
> 
> Jonathan Cameron writes:
>>  Commit message is  somewhat inaccurate...
>>
>> On 09/13/10 20:47, Joe Perches wrote:
>>> Signed-off-by: Joe Perches <joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>
>>> ---
>>>  drivers/i2c/busses/i2c-stu300.c |    4 ++--
>>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/i2c/busses/i2c-stu300.c b/drivers/i2c/busses/i2c-stu300.c
>>> index 495be45..2f7c09c 100644
>>> --- a/drivers/i2c/busses/i2c-stu300.c
>>> +++ b/drivers/i2c/busses/i2c-stu300.c
>>> @@ -871,7 +871,7 @@ stu300_probe(struct platform_device *pdev)
>>>  	struct resource *res;
>>>  	int bus_nr;
>>>  	int ret = 0;
>>> -	char clk_name[] = "I2C0";
>>> +	char clk_name[sizeof("I2Cx")];
>>>  
>>>  	dev = kzalloc(sizeof(struct stu300_dev), GFP_KERNEL);
>>>  	if (!dev) {
>>> @@ -881,7 +881,7 @@ stu300_probe(struct platform_device *pdev)
>>>  	}
>>>  
>>>  	bus_nr = pdev->id;
>>> -	clk_name[3] += (char)bus_nr;
>>> +	sprintf(clk_name, "I2C%c", '0' + bus_nr);
>> I'm guessing that there are never more than a couple of these.
>> Why is this method a better bet than just putting %d?
>>
> '%c' will only ever produce one byte of output while '%d' may
> produce up to 11 bytes depending on the value of bus_nr thus
> overflowing the buffer.
Then use an snprintf, or apply a check to ensure it isn't bigger than
9.

If you don't mind having clocks named i2c$ or something equally
silly then I guess this is fine.  To my mind, if that is possible
this is a bug that should be fixed rather than avoided.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 05/25] drivers/i2c: Use static const char arrays
       [not found]               ` <4C8F7157.9060005-tko9wxEg+fIOOJlXag/Snyp2UmYkHbXO@public.gmane.org>
@ 2010-09-14 19:48                 ` Russell King - ARM Linux
  0 siblings, 0 replies; 10+ messages in thread
From: Russell King - ARM Linux @ 2010-09-14 19:48 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Lothar Waßmann, Linus Walleij,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Jonathan Cameron,
	Jean Delvare (PC drivers, core), linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	Ben Dooks (embedded platforms), Joe Perches,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On Tue, Sep 14, 2010 at 01:57:59PM +0100, Jonathan Cameron wrote:
> On 09/14/10 08:36, Lothar Waßmann wrote:
> > Hi,
> > 
> > Jonathan Cameron writes:
> >>  Commit message is  somewhat inaccurate...
> >>
> >> On 09/13/10 20:47, Joe Perches wrote:
> >>> Signed-off-by: Joe Perches <joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>
> >>> ---
> >>>  drivers/i2c/busses/i2c-stu300.c |    4 ++--
> >>>  1 files changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/i2c/busses/i2c-stu300.c b/drivers/i2c/busses/i2c-stu300.c
> >>> index 495be45..2f7c09c 100644
> >>> --- a/drivers/i2c/busses/i2c-stu300.c
> >>> +++ b/drivers/i2c/busses/i2c-stu300.c
> >>> @@ -871,7 +871,7 @@ stu300_probe(struct platform_device *pdev)
> >>>  	struct resource *res;
> >>>  	int bus_nr;
> >>>  	int ret = 0;
> >>> -	char clk_name[] = "I2C0";
> >>> +	char clk_name[sizeof("I2Cx")];
> >>>  
> >>>  	dev = kzalloc(sizeof(struct stu300_dev), GFP_KERNEL);
> >>>  	if (!dev) {
> >>> @@ -881,7 +881,7 @@ stu300_probe(struct platform_device *pdev)
> >>>  	}
> >>>  
> >>>  	bus_nr = pdev->id;
> >>> -	clk_name[3] += (char)bus_nr;
> >>> +	sprintf(clk_name, "I2C%c", '0' + bus_nr);
> >> I'm guessing that there are never more than a couple of these.
> >> Why is this method a better bet than just putting %d?
> >>
> > '%c' will only ever produce one byte of output while '%d' may
> > produce up to 11 bytes depending on the value of bus_nr thus
> > overflowing the buffer.
> Then use an snprintf, or apply a check to ensure it isn't bigger than
> 9.
> 
> If you don't mind having clocks named i2c$ or something equally
> silly then I guess this is fine.  To my mind, if that is possible
> this is a bug that should be fixed rather than avoided.

Even better, fix the implementation so that it conforms to the API
spec rather than doing its own thing with the consumer connection ID
argument and ignoring the struct device argument.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 05/25] drivers/i2c: Use static const char arrays
       [not found]     ` <b0c83cb1b42c6adf38f6322066fb457693f7ac3b.1284406638.git.joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>
  2010-09-13 19:56       ` Russell King - ARM Linux
  2010-09-13 20:08       ` Jonathan Cameron
@ 2010-09-14 23:23       ` Ben Dooks
  2 siblings, 0 replies; 10+ messages in thread
From: Ben Dooks @ 2010-09-14 23:23 UTC (permalink / raw)
  To: Joe Perches
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Linus Walleij,
	Jean Delvare (PC drivers, core), Ben Dooks (embedded platforms),
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA

On Mon, Sep 13, 2010 at 12:47:43PM -0700, Joe Perches wrote:
> Signed-off-by: Joe Perches <joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>
> ---
>  drivers/i2c/busses/i2c-stu300.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-stu300.c b/drivers/i2c/busses/i2c-stu300.c
> index 495be45..2f7c09c 100644
> --- a/drivers/i2c/busses/i2c-stu300.c
> +++ b/drivers/i2c/busses/i2c-stu300.c
> @@ -871,7 +871,7 @@ stu300_probe(struct platform_device *pdev)
>  	struct resource *res;
>  	int bus_nr;
>  	int ret = 0;
> -	char clk_name[] = "I2C0";
> +	char clk_name[sizeof("I2Cx")];
>  
>  	dev = kzalloc(sizeof(struct stu300_dev), GFP_KERNEL);
>  	if (!dev) {
> @@ -881,7 +881,7 @@ stu300_probe(struct platform_device *pdev)
>  	}
>  
>  	bus_nr = pdev->id;
> -	clk_name[3] += (char)bus_nr;
> +	sprintf(clk_name, "I2C%c", '0' + bus_nr);
>  	dev->clk = clk_get(&pdev->dev, clk_name);
>  	if (IS_ERR(dev->clk)) {
>  		ret = PTR_ERR(dev->clk);

personally, i'd prefer to see snprintf() used, to ensure that this sort
of thing never goes wrong if people decde to change the string. It is
a pitty there's no good way of temporarily allocating data an having it
freed in C.

As such, apologies to Russell about the abuse of the clk_get() call, but
let's not fix that just now.

-- 
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2010-09-14 23:23 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-09-13 19:47 [PATCH 00/25] treewide-next: Use static const char arrays Joe Perches
     [not found] ` <cover.1284406638.git.joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>
2010-09-13 19:47   ` [PATCH 05/25] drivers/i2c: " Joe Perches
     [not found]     ` <b0c83cb1b42c6adf38f6322066fb457693f7ac3b.1284406638.git.joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>
2010-09-13 19:56       ` Russell King - ARM Linux
2010-09-13 20:08       ` Jonathan Cameron
     [not found]         ` <4C8E84A0.1010606-tko9wxEg+fIOOJlXag/Snyp2UmYkHbXO@public.gmane.org>
2010-09-13 20:16           ` Joe Perches
2010-09-14  7:36         ` Lothar Waßmann
     [not found]           ` <19599.9719.562406.692216-VjFSrY7JcPWvSplVBqRQBQ@public.gmane.org>
2010-09-14 12:57             ` Jonathan Cameron
     [not found]               ` <4C8F7157.9060005-tko9wxEg+fIOOJlXag/Snyp2UmYkHbXO@public.gmane.org>
2010-09-14 19:48                 ` Russell King - ARM Linux
2010-09-14 23:23       ` Ben Dooks
2010-09-14  9:14 ` (unknown) David Howells

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).