From: Jani Nikula <jani.nikula@linux.intel.com>
To: "Gustavo A. R. Silva" <gustavo@embeddedor.com>,
Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: David Airlie <airlied@linux.ie>,
intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org,
dri-devel@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH] drm/i915: mark expected switch fall-through
Date: Thu, 21 Jun 2018 11:03:14 +0300 [thread overview]
Message-ID: <87r2l0va65.fsf@intel.com> (raw)
In-Reply-To: <26740470-d7b7-0a58-0aae-fc628bd4f416@embeddedor.com>
On Wed, 20 Jun 2018, "Gustavo A. R. Silva" <gustavo@embeddedor.com> wrote:
> On 06/20/2018 02:06 PM, Rodrigo Vivi wrote:
>> On Wed, Jun 20, 2018 at 08:31:00AM -0500, Gustavo A. R. Silva wrote:
>>> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
>>> where we are expecting to fall through.
>>>
>>> Addresses-Coverity-ID: 1470102 ("Missing break in switch")
>>
>> Any other advantage besides coverity?
>> Can't we address it by marking as "Intentional" on the tool?
>>
>
> Yes. The advantage of this is that it will eventually allows to enable
> -Wimplicit-fallthrough, hence, enabling the compiler to trigger a
> warning, which will force us to double check if we are actually missing
> a break before committing the code.
I applaud the efforts. Since you're doing the comment changes, do you
have an idea what -Wimplicit-fallthrough=N level is being considered for
kernel?
>> I'm afraid there will be so many more places to add fallthrough
>> marks....
>>
>
> Oh yeah, there are around 1000 similar places in the whole codebase.
> There is an ongoing effort to review each case. Months ago, it used to
> be around 1500 of these cases.
We use our own MISSING_CASE() to indicate stuff that's not supposed to
happen, or to be implemented, etc. and in many cases the fallthrough is
normal. I wonder if we could embed __attribute__ ((fallthrough)) in
there to tackle all of these without a comment.
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
WARNING: multiple messages have this Message-ID (diff)
From: Jani Nikula <jani.nikula@linux.intel.com>
To: "Gustavo A. R. Silva" <gustavo@embeddedor.com>,
Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
David Airlie <airlied@linux.ie>,
intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org,
dri-devel@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH] drm/i915: mark expected switch fall-through
Date: Thu, 21 Jun 2018 11:03:14 +0300 [thread overview]
Message-ID: <87r2l0va65.fsf@intel.com> (raw)
In-Reply-To: <26740470-d7b7-0a58-0aae-fc628bd4f416@embeddedor.com>
On Wed, 20 Jun 2018, "Gustavo A. R. Silva" <gustavo@embeddedor.com> wrote:
> On 06/20/2018 02:06 PM, Rodrigo Vivi wrote:
>> On Wed, Jun 20, 2018 at 08:31:00AM -0500, Gustavo A. R. Silva wrote:
>>> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
>>> where we are expecting to fall through.
>>>
>>> Addresses-Coverity-ID: 1470102 ("Missing break in switch")
>>
>> Any other advantage besides coverity?
>> Can't we address it by marking as "Intentional" on the tool?
>>
>
> Yes. The advantage of this is that it will eventually allows to enable
> -Wimplicit-fallthrough, hence, enabling the compiler to trigger a
> warning, which will force us to double check if we are actually missing
> a break before committing the code.
I applaud the efforts. Since you're doing the comment changes, do you
have an idea what -Wimplicit-fallthrough=N level is being considered for
kernel?
>> I'm afraid there will be so many more places to add fallthrough
>> marks....
>>
>
> Oh yeah, there are around 1000 similar places in the whole codebase.
> There is an ongoing effort to review each case. Months ago, it used to
> be around 1500 of these cases.
We use our own MISSING_CASE() to indicate stuff that's not supposed to
happen, or to be implemented, etc. and in many cases the fallthrough is
normal. I wonder if we could embed __attribute__ ((fallthrough)) in
there to tackle all of these without a comment.
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
next prev parent reply other threads:[~2018-06-21 8:03 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-06-20 13:31 [PATCH] drm/i915: mark expected switch fall-through Gustavo A. R. Silva
2018-06-20 15:20 ` ✓ Fi.CI.BAT: success for " Patchwork
2018-06-20 19:06 ` [Intel-gfx] [PATCH] " Rodrigo Vivi
2018-06-20 19:06 ` Rodrigo Vivi
2018-06-20 21:27 ` Gustavo A. R. Silva
2018-06-21 8:03 ` Jani Nikula [this message]
2018-06-21 8:03 ` Jani Nikula
2018-06-27 0:43 ` Gustavo A. R. Silva
2018-06-27 0:43 ` [Intel-gfx] " Gustavo A. R. Silva
2018-06-27 9:30 ` Jani Nikula
2018-06-27 9:30 ` [Intel-gfx] " Jani Nikula
2018-06-28 22:32 ` Gustavo A. R. Silva
2018-06-20 19:31 ` ✓ Fi.CI.IGT: success for " Patchwork
2018-06-27 1:00 ` ✗ Fi.CI.BAT: failure for drm/i915: mark expected switch fall-through (rev2) Patchwork
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=87r2l0va65.fsf@intel.com \
--to=jani.nikula@linux.intel.com \
--cc=airlied@linux.ie \
--cc=dri-devel@lists.freedesktop.org \
--cc=gustavo@embeddedor.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=linux-kernel@vger.kernel.org \
--cc=rodrigo.vivi@intel.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.