Linux-Media Archive on lore.kernel.org
 help / color / Atom feed
From: Abylay Ospan <aospan@netup.ru>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Sergey Kozlov <serjk@netup.ru>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Jason Baron <jbaron@akamai.com>,
	linux-media <linux-media@vger.kernel.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] [media] cxd2841er: avoid misleading gcc warning
Date: Mon, 25 Jul 2016 00:24:27 -0400
Message-ID: <CAK3bHNXrkqxpwxsL1BE93gw84aAHrxVa+c8m+s9JpQbFwP=rog@mail.gmail.com> (raw)
In-Reply-To: <20160713204342.1221511-1-arnd@arndb.de>

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

      reply index

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-13 20:42 Arnd Bergmann
2016-07-25  4:24 ` Abylay Ospan [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAK3bHNXrkqxpwxsL1BE93gw84aAHrxVa+c8m+s9JpQbFwP=rog@mail.gmail.com' \
    --to=aospan@netup.ru \
    --cc=arnd@arndb.de \
    --cc=jbaron@akamai.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=serjk@netup.ru \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Media Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-media/0 linux-media/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-media linux-media/ https://lore.kernel.org/linux-media \
		linux-media@vger.kernel.org
	public-inbox-index linux-media

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-media


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git