Re: [kbuild-all] [linuxtv-media:master 499/499] drivers/media/i2c/s5k5baf.c:362:3: warning: format '%d' expects argument of type 'int', but argument 3 has type 'size_t'

[Mauro Carvalho Chehab <m.chehab@samsung.com>]

Em Wed, 08 Jan 2014 14:33:30 +0100
Andrzej Hajda <a.hajda@samsung.com> escreveu:

> On 01/08/2014 01:21 PM, Mauro Carvalho Chehab wrote:
> > Em Wed, 8 Jan 2014 11:37:37 +0300
> > Dan Carpenter <dan.carpenter@oracle.com> escreveu:
> >
> >> The other thing that concerned me with this was the sparse warning:
> >>
> >> drivers/media/i2c/s5k5baf.c:481:26: error: bad constant expression
> > Hmm...
> > 	static void s5k5baf_write_arr_seq(struct s5k5baf *state, u16 addr,
> > 	                                  u16 count, const u16 *seq)
> > 	{
> > 	        struct i2c_client *c = v4l2_get_subdevdata(&state->sd);
> > 	        __be16 buf[count + 1];
> > 	        int ret, n;
> >
> > Yeah, allocating data like that at stack is not nice.
> >
> > I would simply replace the static allocation here by a dynamic one.
> Sequences are very short (usually few words) and their length is known
> in compile time.
> The only exception are sequences provided by firmware file and for them
> I can add check in s5k5baf_write_nseq to make it safe.
> 
> Replacing it with dynamic allocation seems to me unnecessary in this
> particular case, it would result in memory allocation/free for every
> single access to
> the device. What do you think?

Well, if you know in advance what's the maximum size, just replace it by
something like:

	__be16 buf[64];

and check if count + 1 is less or equal to sizeof(buf).

As the kernel stack is really small, we should avoid allocating large
data there (1KB is large, on that sense).

Also, it would be possible o use i2cdev to inject very large payloads
to be sent to a random I2C device. That could cause a machine OOPS
or to use it to inject a security breach code.

So, we should really avoid using dynamic static allocation, specially
on I2C handlers.

Regards,
Mauro