* [PATCHv2 0/3] Three fixes to clean up daily build warnings
@ 2020-10-08 11:59 Hans Verkuil
2020-10-08 11:59 ` [PATCHv2 1/3] s5k5baf: drop 'data' field in struct s5k5baf_fw Hans Verkuil
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Hans Verkuil @ 2020-10-08 11:59 UTC (permalink / raw)
To: linux-media
New warnings appeared in the daily build after switching to gcc 10.2.0.
These patches fix those warnings.
Changes since v1: incorporated irc comments from Mauro
- tvp7002.c: rather than initializing val, just don't fill 'reg' on error.
- drxk_hard.c: the underlying cause is a wrong while condition, fix that.
Hans
Hans Verkuil (3):
s5k5baf: drop 'data' field in struct s5k5baf_fw
tvp7002: fix uninitialized variable warning
dvb-frontends/drxk_hard.c: fix uninitialized variable warning
drivers/media/dvb-frontends/drxk_hard.c | 3 +--
drivers/media/i2c/s5k5baf.c | 5 ++---
drivers/media/i2c/tvp7002.c | 4 +++-
3 files changed, 6 insertions(+), 6 deletions(-)
--
2.28.0
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCHv2 1/3] s5k5baf: drop 'data' field in struct s5k5baf_fw
2020-10-08 11:59 [PATCHv2 0/3] Three fixes to clean up daily build warnings Hans Verkuil
@ 2020-10-08 11:59 ` Hans Verkuil
2020-10-08 12:58 ` Philipp Zabel
2020-10-08 11:59 ` [PATCHv2 2/3] tvp7002: fix uninitialized variable warning Hans Verkuil
2020-10-08 11:59 ` [PATCHv2 3/3] dvb-frontends/drxk_hard.c: " Hans Verkuil
2 siblings, 1 reply; 6+ messages in thread
From: Hans Verkuil @ 2020-10-08 11:59 UTC (permalink / raw)
To: linux-media; +Cc: Hans Verkuil, Kyungmin Park, Sylwester Nawrocki
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;
for (i = 0; i < fw->count; ++i) {
if (fw->seq[i].id == seq_id)
--
2.28.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCHv2 1/3] s5k5baf: drop 'data' field in struct s5k5baf_fw
2020-10-08 11:59 ` [PATCHv2 1/3] s5k5baf: drop 'data' field in struct s5k5baf_fw Hans Verkuil
@ 2020-10-08 12:58 ` Philipp Zabel
2020-10-08 14:01 ` Philipp Zabel
0 siblings, 1 reply; 6+ messages in thread
From: Philipp Zabel @ 2020-10-08 12:58 UTC (permalink / raw)
To: Hans Verkuil, linux-media; +Cc: Kyungmin Park, Sylwester Nawrocki
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCHv2 1/3] s5k5baf: drop 'data' field in struct s5k5baf_fw
2020-10-08 12:58 ` Philipp Zabel
@ 2020-10-08 14:01 ` Philipp Zabel
0 siblings, 0 replies; 6+ messages in thread
From: Philipp Zabel @ 2020-10-08 14:01 UTC (permalink / raw)
To: Hans Verkuil, linux-media; +Cc: Kyungmin Park, Sylwester Nawrocki
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCHv2 2/3] tvp7002: fix uninitialized variable warning
2020-10-08 11:59 [PATCHv2 0/3] Three fixes to clean up daily build warnings Hans Verkuil
2020-10-08 11:59 ` [PATCHv2 1/3] s5k5baf: drop 'data' field in struct s5k5baf_fw Hans Verkuil
@ 2020-10-08 11:59 ` Hans Verkuil
2020-10-08 11:59 ` [PATCHv2 3/3] dvb-frontends/drxk_hard.c: " Hans Verkuil
2 siblings, 0 replies; 6+ messages in thread
From: Hans Verkuil @ 2020-10-08 11:59 UTC (permalink / raw)
To: linux-media; +Cc: Hans Verkuil
tvp7002.c: In function 'tvp7002_g_register':
tvp7002.c:691:11: warning: 'val' may be used uninitialized in this function [-Wmaybe-uninitialized]
691 | reg->val = val;
| ~~~~~~~~~^~~~~
Just return without setting 'reg' if tvp7002_read returns an error.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
drivers/media/i2c/tvp7002.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/media/i2c/tvp7002.c b/drivers/media/i2c/tvp7002.c
index de313b1306da..ada4ec5ef782 100644
--- a/drivers/media/i2c/tvp7002.c
+++ b/drivers/media/i2c/tvp7002.c
@@ -688,9 +688,11 @@ static int tvp7002_g_register(struct v4l2_subdev *sd,
int ret;
ret = tvp7002_read(sd, reg->reg & 0xff, &val);
+ if (ret < 0)
+ return ret;
reg->val = val;
reg->size = 1;
- return ret;
+ return 0;
}
/*
--
2.28.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCHv2 3/3] dvb-frontends/drxk_hard.c: fix uninitialized variable warning
2020-10-08 11:59 [PATCHv2 0/3] Three fixes to clean up daily build warnings Hans Verkuil
2020-10-08 11:59 ` [PATCHv2 1/3] s5k5baf: drop 'data' field in struct s5k5baf_fw Hans Verkuil
2020-10-08 11:59 ` [PATCHv2 2/3] tvp7002: fix uninitialized variable warning Hans Verkuil
@ 2020-10-08 11:59 ` Hans Verkuil
2 siblings, 0 replies; 6+ messages in thread
From: Hans Verkuil @ 2020-10-08 11:59 UTC (permalink / raw)
To: linux-media; +Cc: Hans Verkuil
drxk_hard.c: In function 'hi_command.constprop':
drxk_hard.c:1016:5: warning: 'wait_cmd' may be used uninitialized in this function [-Wmaybe-uninitialized]
1015 | } while ((status < 0) && (retry_count < DRXK_MAX_RETRIES)
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1016 | && (wait_cmd != 0));
| ^~~~~~~~~~~~~~~~~~
The underlying cause is that the while condition is wrong. It should be:
(status < 0 || wait_cmd) && (retry_count < DRXK_MAX_RETRIES)
'wait_cmd' is only valid if '!(status < 0)'.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
drivers/media/dvb-frontends/drxk_hard.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/media/dvb-frontends/drxk_hard.c b/drivers/media/dvb-frontends/drxk_hard.c
index 32f9346deb3e..a57470bf71bf 100644
--- a/drivers/media/dvb-frontends/drxk_hard.c
+++ b/drivers/media/dvb-frontends/drxk_hard.c
@@ -1011,8 +1011,7 @@ static int hi_command(struct drxk_state *state, u16 cmd, u16 *p_result)
retry_count += 1;
status = read16(state, SIO_HI_RA_RAM_CMD__A,
&wait_cmd);
- } while ((status < 0) && (retry_count < DRXK_MAX_RETRIES)
- && (wait_cmd != 0));
+ } while ((status < 0 || wait_cmd) && (retry_count < DRXK_MAX_RETRIES));
if (status < 0)
goto error;
status = read16(state, SIO_HI_RA_RAM_RES__A, p_result);
--
2.28.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-10-08 14:01 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-08 11:59 [PATCHv2 0/3] Three fixes to clean up daily build warnings Hans Verkuil
2020-10-08 11:59 ` [PATCHv2 1/3] s5k5baf: drop 'data' field in struct s5k5baf_fw Hans Verkuil
2020-10-08 12:58 ` Philipp Zabel
2020-10-08 14:01 ` Philipp Zabel
2020-10-08 11:59 ` [PATCHv2 2/3] tvp7002: fix uninitialized variable warning Hans Verkuil
2020-10-08 11:59 ` [PATCHv2 3/3] dvb-frontends/drxk_hard.c: " Hans Verkuil
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).