Re: [PATCH 05/25] drivers/i2c: Use static const char arrays

[Russell King - ARM Linux <linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>]

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.