In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through.
Addresses-Coverity-ID: 141432 Addresses-Coverity-ID: 141433 Addresses-Coverity-ID: 141434 Addresses-Coverity-ID: 141435 Addresses-Coverity-ID: 141436 Addresses-Coverity-ID: 1357360 Addresses-Coverity-ID: 1357403 Addresses-Coverity-ID: 1357433 Addresses-Coverity-ID: 1392622 Addresses-Coverity-ID: 1415273 Addresses-Coverity-ID: 1435752 Addresses-Coverity-ID: 1441500 Addresses-Coverity-ID: 1454596 Signed-off-by: Gustavo A. R. Silva garsilva@embeddedor.com --- drivers/gpu/drm/i915/i915_gem.c | 3 ++- drivers/gpu/drm/i915/intel_cdclk.c | 4 ++++ drivers/gpu/drm/i915/intel_display.c | 2 ++ drivers/gpu/drm/i915/intel_drv.h | 1 + drivers/gpu/drm/i915/intel_engine_cs.c | 2 ++ drivers/gpu/drm/i915/intel_runtime_pm.c | 1 + drivers/gpu/drm/i915/intel_sdvo.c | 6 ++++++ 7 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 3a140ee..326cf16 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1977,6 +1977,7 @@ int i915_gem_fault(struct vm_fault *vmf) ret = VM_FAULT_SIGBUS; break; } + /* fall through */ case -EAGAIN: /* * EAGAIN means the gpu is hung and we'll wait for the error @@ -2644,7 +2645,7 @@ static void *i915_gem_object_map(const struct drm_i915_gem_object *obj, switch (type) { default: MISSING_CASE(type); - /* fallthrough to use PAGE_KERNEL anyway */ + /* fall through - To use PAGE_KERNEL anyway */ case I915_MAP_WB: pgprot = PAGE_KERNEL; break; diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c index b2a6d62..5879cd3 100644 --- a/drivers/gpu/drm/i915/intel_cdclk.c +++ b/drivers/gpu/drm/i915/intel_cdclk.c @@ -316,6 +316,7 @@ static void pnv_get_cdclk(struct drm_i915_private *dev_priv, break; default: DRM_ERROR("Unknown pnv display core clock 0x%04x\n", gcfgc); + /* fall through */ case GC_DISPLAY_CLOCK_133_MHZ_PNV: cdclk_state->cdclk = 133333; break; @@ -1110,6 +1111,7 @@ static int bxt_de_pll_vco(struct drm_i915_private *dev_priv, int cdclk) switch (cdclk) { default: MISSING_CASE(cdclk); + /* fall through */ case 144000: case 288000: case 384000: @@ -1134,6 +1136,7 @@ static int glk_de_pll_vco(struct drm_i915_private *dev_priv, int cdclk) switch (cdclk) { default: MISSING_CASE(cdclk); + /* fall through */ case 79200: case 158400: case 316800: @@ -1592,6 +1595,7 @@ static int cnl_cdclk_pll_vco(struct drm_i915_private *dev_priv, int cdclk) switch (cdclk) { default: MISSING_CASE(cdclk); + /* fall through */ case 168000: case 336000: ratio = dev_priv->cdclk.hw.ref == 19200 ? 35 : 28; diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 878acc4..1f7907f 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9049,6 +9049,7 @@ static bool hsw_get_transcoder_state(struct intel_crtc *crtc, switch (tmp & TRANS_DDI_EDP_INPUT_MASK) { default: WARN(1, "unknown pipe linked to edp transcoder\n"); + /* fall through */ case TRANS_DDI_EDP_INPUT_A_ONOFF: case TRANS_DDI_EDP_INPUT_A_ON: trans_edp_pipe = PIPE_A; @@ -10751,6 +10752,7 @@ static bool check_digital_port_conflicts(struct drm_atomic_state *state) case INTEL_OUTPUT_UNKNOWN: if (WARN_ON(!HAS_DDI(to_i915(dev)))) break; + /* fall through */ case INTEL_OUTPUT_DP: case INTEL_OUTPUT_HDMI: case INTEL_OUTPUT_EDP: diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 7bc60c8..4c3696c 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1149,6 +1149,7 @@ enc_to_dig_port(struct drm_encoder *encoder) switch (intel_encoder->type) { case INTEL_OUTPUT_UNKNOWN: WARN_ON(!HAS_DDI(to_i915(encoder->dev))); + /* fall through */ case INTEL_OUTPUT_DP: case INTEL_OUTPUT_EDP: case INTEL_OUTPUT_HDMI: diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index ab5bf4e..d02f37d 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -153,6 +153,7 @@ __intel_engine_context_size(struct drm_i915_private *dev_priv, u8 class) switch (INTEL_GEN(dev_priv)) { default: MISSING_CASE(INTEL_GEN(dev_priv)); + /* fall through */ case 10: return GEN10_LR_CONTEXT_RENDER_SIZE; case 9: @@ -183,6 +184,7 @@ __intel_engine_context_size(struct drm_i915_private *dev_priv, u8 class) break; default: MISSING_CASE(class); + /* fall through */ case VIDEO_DECODE_CLASS: case VIDEO_ENHANCEMENT_CLASS: case COPY_ENGINE_CLASS: diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c index 8af286c..c60dc67 100644 --- a/drivers/gpu/drm/i915/intel_runtime_pm.c +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c @@ -2740,6 +2740,7 @@ static void cnl_set_procmon_ref_values(struct drm_i915_private *dev_priv) switch (val & (PROCESS_INFO_MASK | VOLTAGE_INFO_MASK)) { default: MISSING_CASE(val); + /* fall through */ case VOLTAGE_INFO_0_85V | PROCESS_INFO_DOT_0: procmon = &cnl_procmon_values[PROCMON_0_85V_DOT_0]; break; diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c index 7437944..3860438 100644 --- a/drivers/gpu/drm/i915/intel_sdvo.c +++ b/drivers/gpu/drm/i915/intel_sdvo.c @@ -1327,6 +1327,7 @@ static void intel_sdvo_pre_enable(struct intel_encoder *intel_encoder, switch (crtc_state->pixel_multiplier) { default: WARN(1, "unknown pixel multiplier specified\n"); + /* fall through */ case 1: rate = SDVO_CLOCK_RATE_MULT_1X; break; case 2: rate = SDVO_CLOCK_RATE_MULT_2X; break; case 4: rate = SDVO_CLOCK_RATE_MULT_4X; break; @@ -2275,14 +2276,19 @@ intel_sdvo_guess_ddc_bus(struct intel_sdvo *sdvo) switch (sdvo->controlled_output) { case SDVO_OUTPUT_LVDS1: mask |= SDVO_OUTPUT_LVDS1; + /* fall through */ case SDVO_OUTPUT_LVDS0: mask |= SDVO_OUTPUT_LVDS0; + /* fall through */ case SDVO_OUTPUT_TMDS1: mask |= SDVO_OUTPUT_TMDS1; + /* fall through */ case SDVO_OUTPUT_TMDS0: mask |= SDVO_OUTPUT_TMDS0; + /* fall through */ case SDVO_OUTPUT_RGB1: mask |= SDVO_OUTPUT_RGB1; + /* fall through */ case SDVO_OUTPUT_RGB0: mask |= SDVO_OUTPUT_RGB0; break;
On Mon, 2017-11-27 at 16:17 -0600, Gustavo A. R. Silva wrote:
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through.
I have to say I'm totally not sold on regexps matching comment contents. Was something more explicit ever considered? Like:
#define FALLTHROUGH __attribute__((fallthrough));
With the appropriate version checks, of course.
Regards, Joonas
Hi Joonas,
Quoting Joonas Lahtinen joonas.lahtinen@linux.intel.com:
On Mon, 2017-11-27 at 16:17 -0600, Gustavo A. R. Silva wrote:
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through.
I have to say I'm totally not sold on regexps matching comment contents. Was something more explicit ever considered? Like:
#define FALLTHROUGH __attribute__((fallthrough));
With the appropriate version checks, of course.
One of the arguments is that comments lets us leverage the existing static analyzers.
We've been discussing this during the last week, feel free to join the discussion:
http://www.spinics.net/lists/kernel/msg2659908.html http://www.spinics.net/lists/kernel/msg2659906.html
Thanks! -- Gustavo A. R. Silva
On Mon, Dec 4, 2017 at 4:20 PM, Gustavo A. R. Silva garsilva@embeddedor.com wrote:
Hi Joonas,
Quoting Joonas Lahtinen joonas.lahtinen@linux.intel.com:
On Mon, 2017-11-27 at 16:17 -0600, Gustavo A. R. Silva wrote:
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through.
I have to say I'm totally not sold on regexps matching comment contents. Was something more explicit ever considered? Like:
#define FALLTHROUGH __attribute__((fallthrough));
With the appropriate version checks, of course.
One of the arguments is that comments lets us leverage the existing static analyzers.
We've been discussing this during the last week, feel free to join the discussion:
http://www.spinics.net/lists/kernel/msg2659908.html http://www.spinics.net/lists/kernel/msg2659906.html
If we go with existing rules, then either pls patch coding style, or be a bit more liberal in what you accept. E.g. fallthrough vs fall through seems a bit a bikeshed (and will be an endless source of work for you).
I'd also claim that "this shouldn't happen, dump a backtrace and hope for the best" style macros like i915's MISSING_CASE or WARN_ON (as the only thing) should count as an auto-fallthrough annotation.
From a quick look, that would cover everything in your patch.
-Daniel
Thanks!
Gustavo A. R. Silva
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
dri-devel@lists.freedesktop.org