From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Kees Cook <keescook@chromium.org>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
Maxime Ripard <mripard@kernel.org>,
David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>,
dri-devel@lists.freedesktop.org,
Alexander Potapenko <glider@google.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] drm/edid: Distribute switch variables for initialization
Date: Tue, 3 Mar 2020 20:01:59 +0200 [thread overview]
Message-ID: <20200303180159.GA13686@intel.com> (raw)
In-Reply-To: <202003022038.07A611E@keescook>
On Mon, Mar 02, 2020 at 08:39:37PM -0800, Kees Cook wrote:
> On Wed, Feb 19, 2020 at 10:22:29PM -0800, Kees Cook wrote:
> > Variables declared in a switch statement before any case statements
> > cannot be automatically initialized with compiler instrumentation (as
> > they are not part of any execution flow). With GCC's proposed automatic
> > stack variable initialization feature, this triggers a warning (and they
> > don't get initialized). Clang's automatic stack variable initialization
> > (via CONFIG_INIT_STACK_ALL=y) doesn't throw a warning, but it also
> > doesn't initialize such variables[1]. Note that these warnings (or silent
> > skipping) happen before the dead-store elimination optimization phase,
> > so even when the automatic initializations are later elided in favor of
> > direct initializations, the warnings remain.
> >
> > To avoid these problems, move such variables into the "case" where
> > they're used or lift them up into the main function body.
> >
> > drivers/gpu/drm/drm_edid.c: In function ‘drm_edid_to_eld’:
> > drivers/gpu/drm/drm_edid.c:4395:9: warning: statement will never be executed [-Wswitch-unreachable]
> > 4395 | int sad_count;
> > | ^~~~~~~~~
> >
> > [1] https://bugs.llvm.org/show_bug.cgi?id=44916
> >
> > Signed-off-by: Kees Cook <keescook@chromium.org>
>
> Ping. Can someone pick this up, please?
>
> Thanks!
>
> -Kees
>
> > ---
> > drivers/gpu/drm/drm_edid.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index 805fb004c8eb..2941b65b427f 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -4392,9 +4392,9 @@ static void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid)
> > dbl = cea_db_payload_len(db);
> >
> > switch (cea_db_tag(db)) {
> > - int sad_count;
> > + case AUDIO_BLOCK: {
I've never been a fan of {} inside switch statements. I'd just
move this one level up.
> >
> > - case AUDIO_BLOCK:
> > + int sad_count;
> > /* Audio Data Block, contains SADs */
> > sad_count = min(dbl / 3, 15 - total_sad_count);
> > if (sad_count >= 1)
> > @@ -4402,6 +4402,7 @@ static void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid)
> > &db[1], sad_count * 3);
> > total_sad_count += sad_count;
> > break;
> > + }
> > case SPEAKER_BLOCK:
> > /* Speaker Allocation Data Block */
> > if (dbl >= 1)
> >
>
> --
> Kees Cook
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrjälä
Intel
prev parent reply other threads:[~2020-03-03 18:02 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-20 6:22 [PATCH] drm/edid: Distribute switch variables for initialization Kees Cook
2020-03-03 4:39 ` Kees Cook
2020-03-03 8:21 ` Daniel Vetter
2020-03-03 18:01 ` Ville Syrjälä [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=20200303180159.GA13686@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=airlied@linux.ie \
--cc=daniel@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=glider@google.com \
--cc=keescook@chromium.org \
--cc=linux-kernel@vger.kernel.org \
--cc=maarten.lankhorst@linux.intel.com \
--cc=mripard@kernel.org \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).