* [PATCH] [media] cxd2841er: avoid misleading gcc warning
@ 2016-07-13 20:42 Arnd Bergmann
2016-07-25 4:24 ` Abylay Ospan
0 siblings, 1 reply; 2+ messages in thread
From: Arnd Bergmann @ 2016-07-13 20:42 UTC (permalink / raw)
To: Sergey Kozlov, Abylay Ospan, Mauro Carvalho Chehab
Cc: Jason Baron, Arnd Bergmann, linux-media, linux-kernel
The addition of jump label support in dynamic_debug caused an unexpected
warning in exactly one file in the kernel:
drivers/media/dvb-frontends/cxd2841er.c: In function 'cxd2841er_tune_tc':
include/linux/dynamic_debug.h:134:3: error: 'carrier_offset' may be used uninitialized in this function [-Werror=maybe-uninitialized]
__dynamic_dev_dbg(&descriptor, dev, fmt, \
^~~~~~~~~~~~~~~~~
drivers/media/dvb-frontends/cxd2841er.c:3177:11: note: 'carrier_offset' was declared here
int ret, carrier_offset;
^~~~~~~~~~~~~~
The problem seems to be that the compiler gets confused by the extra conditionals
in static_branch_unlikely, to the point where it can no longer keep track of
which branches have already been taken, and it doesn't realize that this variable
is now always initialized when it gets used.
I have done lots of randconfig kernel builds and could not find any other file
with this behavior, so I assume it's a rare enough glitch that we don't need
to change the jump label support but instead just work around the warning in
the driver.
To achieve that, I'm moving the check for the return value into the switch()
statement, which is an obvious transformation, but is enough to un-confuse
the compiler here. The resulting code is not as nice to read, but at
least we retain the behavior of warning if it gets changed to actually
access an uninitialized carrier offset value in the future.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: (in linux-mm) "dynamic_debug: add jump label support"
---
drivers/media/dvb-frontends/cxd2841er.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/media/dvb-frontends/cxd2841er.c b/drivers/media/dvb-frontends/cxd2841er.c
index 721fb074da7c..0639ca281a2c 100644
--- a/drivers/media/dvb-frontends/cxd2841er.c
+++ b/drivers/media/dvb-frontends/cxd2841er.c
@@ -3223,20 +3223,28 @@ static int cxd2841er_tune_tc(struct dvb_frontend *fe,
ret = cxd2841er_get_carrier_offset_i(
priv, p->bandwidth_hz,
&carrier_offset);
+ if (ret)
+ return ret;
break;
case SYS_DVBT:
ret = cxd2841er_get_carrier_offset_t(
priv, p->bandwidth_hz,
&carrier_offset);
+ if (ret)
+ return ret;
break;
case SYS_DVBT2:
ret = cxd2841er_get_carrier_offset_t2(
priv, p->bandwidth_hz,
&carrier_offset);
+ if (ret)
+ return ret;
break;
case SYS_DVBC_ANNEX_A:
ret = cxd2841er_get_carrier_offset_c(
priv, &carrier_offset);
+ if (ret)
+ return ret;
break;
default:
dev_dbg(&priv->i2c->dev,
@@ -3244,8 +3252,6 @@ static int cxd2841er_tune_tc(struct dvb_frontend *fe,
__func__, priv->system);
return -EINVAL;
}
- if (ret)
- return ret;
dev_dbg(&priv->i2c->dev, "%s(): carrier offset %d\n",
__func__, carrier_offset);
p->frequency += carrier_offset;
--
2.9.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] [media] cxd2841er: avoid misleading gcc warning
2016-07-13 20:42 [PATCH] [media] cxd2841er: avoid misleading gcc warning Arnd Bergmann
@ 2016-07-25 4:24 ` Abylay Ospan
0 siblings, 0 replies; 2+ messages in thread
From: Abylay Ospan @ 2016-07-25 4:24 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Sergey Kozlov, Mauro Carvalho Chehab, Jason Baron, linux-media,
linux-kernel
Hello Arnd,
thanks for patch. it looks ok.
Acked-by: Abylay Ospan <aospan@netup.ru>
2016-07-13 16:42 GMT-04:00 Arnd Bergmann <arnd@arndb.de>:
> The addition of jump label support in dynamic_debug caused an unexpected
> warning in exactly one file in the kernel:
>
> drivers/media/dvb-frontends/cxd2841er.c: In function 'cxd2841er_tune_tc':
> include/linux/dynamic_debug.h:134:3: error: 'carrier_offset' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> __dynamic_dev_dbg(&descriptor, dev, fmt, \
> ^~~~~~~~~~~~~~~~~
> drivers/media/dvb-frontends/cxd2841er.c:3177:11: note: 'carrier_offset' was declared here
> int ret, carrier_offset;
> ^~~~~~~~~~~~~~
>
> The problem seems to be that the compiler gets confused by the extra conditionals
> in static_branch_unlikely, to the point where it can no longer keep track of
> which branches have already been taken, and it doesn't realize that this variable
> is now always initialized when it gets used.
>
> I have done lots of randconfig kernel builds and could not find any other file
> with this behavior, so I assume it's a rare enough glitch that we don't need
> to change the jump label support but instead just work around the warning in
> the driver.
>
> To achieve that, I'm moving the check for the return value into the switch()
> statement, which is an obvious transformation, but is enough to un-confuse
> the compiler here. The resulting code is not as nice to read, but at
> least we retain the behavior of warning if it gets changed to actually
> access an uninitialized carrier offset value in the future.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: (in linux-mm) "dynamic_debug: add jump label support"
> ---
> drivers/media/dvb-frontends/cxd2841er.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/dvb-frontends/cxd2841er.c b/drivers/media/dvb-frontends/cxd2841er.c
> index 721fb074da7c..0639ca281a2c 100644
> --- a/drivers/media/dvb-frontends/cxd2841er.c
> +++ b/drivers/media/dvb-frontends/cxd2841er.c
> @@ -3223,20 +3223,28 @@ static int cxd2841er_tune_tc(struct dvb_frontend *fe,
> ret = cxd2841er_get_carrier_offset_i(
> priv, p->bandwidth_hz,
> &carrier_offset);
> + if (ret)
> + return ret;
> break;
> case SYS_DVBT:
> ret = cxd2841er_get_carrier_offset_t(
> priv, p->bandwidth_hz,
> &carrier_offset);
> + if (ret)
> + return ret;
> break;
> case SYS_DVBT2:
> ret = cxd2841er_get_carrier_offset_t2(
> priv, p->bandwidth_hz,
> &carrier_offset);
> + if (ret)
> + return ret;
> break;
> case SYS_DVBC_ANNEX_A:
> ret = cxd2841er_get_carrier_offset_c(
> priv, &carrier_offset);
> + if (ret)
> + return ret;
> break;
> default:
> dev_dbg(&priv->i2c->dev,
> @@ -3244,8 +3252,6 @@ static int cxd2841er_tune_tc(struct dvb_frontend *fe,
> __func__, priv->system);
> return -EINVAL;
> }
> - if (ret)
> - return ret;
> dev_dbg(&priv->i2c->dev, "%s(): carrier offset %d\n",
> __func__, carrier_offset);
> p->frequency += carrier_offset;
> --
> 2.9.0
>
--
Abylay Ospan,
NetUP Inc.
http://www.netup.tv
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2016-07-25 4:24 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-13 20:42 [PATCH] [media] cxd2841er: avoid misleading gcc warning Arnd Bergmann
2016-07-25 4:24 ` Abylay Ospan
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).