| Message ID | 20201008115913.3579973-2-hverkuil-cisco@xs4all.nl |
|---|---|
| State | Accepted |
| Commit | 7124ae48f2a45399789bcc14389614c899f6aa7c |
| Headers | show |
| Series | [PATCHv2,1/3] s5k5baf: drop 'data' field in struct s5k5baf_fw | expand |
On Thu, 2020-10-08 at 13:59 +0200, Hans Verkuil wrote: > struct s5k5baf_fw ends with this: > > struct { > u16 id; > u16 offset; > } seq[0]; > u16 data[]; > }; > > which is rather confusing and can cause gcc warnings: > > s5k5baf.c: In function 's5k5baf_load_setfile.isra': > s5k5baf.c:390:13: warning: array subscript 65535 is outside the bounds of an interior zero-length array 'struct <anonymous>[0]' [-Wzero-length-bounds] > 390 | if (f->seq[i].offset + d <= end) > | ~~~~~~^~~ > > It turns out that 'data[]' is used in only one place and it can > easily be replaced by &fw->seq[0].id and 'seq[0]' can be replaced by > 'seq[]'. > > This is both more readable and solved that warnings. > > Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> > Cc: Kyungmin Park <kyungmin.park@samsung.com> > Cc: Sylwester Nawrocki <snawrocki@kernel.org> > --- > drivers/media/i2c/s5k5baf.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/media/i2c/s5k5baf.c b/drivers/media/i2c/s5k5baf.c > index 42584a088273..ec6f22efe19a 100644 > --- a/drivers/media/i2c/s5k5baf.c > +++ b/drivers/media/i2c/s5k5baf.c > @@ -280,8 +280,7 @@ struct s5k5baf_fw { > struct { > u16 id; > u16 offset; > - } seq[0]; > - u16 data[]; > + } seq[]; > }; > > struct s5k5baf { > @@ -563,7 +562,7 @@ static u16 *s5k5baf_fw_get_seq(struct s5k5baf *state, u16 seq_id) > if (fw == NULL) > return NULL; > > - data = fw->data + 2 * fw->count; > + data = &fw->seq[0].id + 2 * fw->count; Would it make sense to make this + data = &fw->seq[fw->count]; or + data = fw->seq + fw->count; instead? regards Philipp
On Thu, 2020-10-08 at 14:58 +0200, Philipp Zabel wrote: > On Thu, 2020-10-08 at 13:59 +0200, Hans Verkuil wrote: > > struct s5k5baf_fw ends with this: > > > > struct { > > u16 id; > > u16 offset; > > } seq[0]; > > u16 data[]; > > }; > > > > which is rather confusing and can cause gcc warnings: > > > > s5k5baf.c: In function 's5k5baf_load_setfile.isra': > > s5k5baf.c:390:13: warning: array subscript 65535 is outside the bounds of an interior zero-length array 'struct <anonymous>[0]' [-Wzero-length-bounds] > > 390 | if (f->seq[i].offset + d <= end) > > | ~~~~~~^~~ > > > > It turns out that 'data[]' is used in only one place and it can > > easily be replaced by &fw->seq[0].id and 'seq[0]' can be replaced by > > 'seq[]'. > > > > This is both more readable and solved that warnings. > > > > Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> > > Cc: Kyungmin Park <kyungmin.park@samsung.com> > > Cc: Sylwester Nawrocki <snawrocki@kernel.org> > > --- > > drivers/media/i2c/s5k5baf.c | 5 ++--- > > 1 file changed, 2 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/media/i2c/s5k5baf.c b/drivers/media/i2c/s5k5baf.c > > index 42584a088273..ec6f22efe19a 100644 > > --- a/drivers/media/i2c/s5k5baf.c > > +++ b/drivers/media/i2c/s5k5baf.c > > @@ -280,8 +280,7 @@ struct s5k5baf_fw { > > struct { > > u16 id; > > u16 offset; > > - } seq[0]; > > - u16 data[]; > > + } seq[]; > > }; > > > > struct s5k5baf { > > @@ -563,7 +562,7 @@ static u16 *s5k5baf_fw_get_seq(struct s5k5baf *state, u16 seq_id) > > if (fw == NULL) > > return NULL; > > > > - data = fw->data + 2 * fw->count; > > + data = &fw->seq[0].id + 2 * fw->count; > > Would it make sense to make this > > + data = &fw->seq[fw->count]; > > or > > + data = fw->seq + fw->count; > > instead? Sorry for the noise, I missed that data is of type (u16 *). regards Philipp
diff --git a/drivers/media/i2c/s5k5baf.c b/drivers/media/i2c/s5k5baf.c index 42584a088273..ec6f22efe19a 100644 --- a/drivers/media/i2c/s5k5baf.c +++ b/drivers/media/i2c/s5k5baf.c @@ -280,8 +280,7 @@ struct s5k5baf_fw { struct { u16 id; u16 offset; - } seq[0]; - u16 data[]; + } seq[]; }; struct s5k5baf { @@ -563,7 +562,7 @@ static u16 *s5k5baf_fw_get_seq(struct s5k5baf *state, u16 seq_id) if (fw == NULL) return NULL; - data = fw->data + 2 * fw->count; + data = &fw->seq[0].id + 2 * fw->count; for (i = 0; i < fw->count; ++i) { if (fw->seq[i].id == seq_id)
struct s5k5baf_fw ends with this: struct { u16 id; u16 offset; } seq[0]; u16 data[]; }; which is rather confusing and can cause gcc warnings: s5k5baf.c: In function 's5k5baf_load_setfile.isra': s5k5baf.c:390:13: warning: array subscript 65535 is outside the bounds of an interior zero-length array 'struct <anonymous>[0]' [-Wzero-length-bounds] 390 | if (f->seq[i].offset + d <= end) | ~~~~~~^~~ It turns out that 'data[]' is used in only one place and it can easily be replaced by &fw->seq[0].id and 'seq[0]' can be replaced by 'seq[]'. This is both more readable and solved that warnings. Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> Cc: Kyungmin Park <kyungmin.park@samsung.com> Cc: Sylwester Nawrocki <snawrocki@kernel.org> --- drivers/media/i2c/s5k5baf.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)