[Intel-gfx] [PATCH] drm/i915: Reword warning for missing cases

Chris Wilson chris at chris-wilson.co.uk
Tue Mar 13 13:06:49 UTC 2018


Quoting Ville Syrjälä (2018-03-13 13:01:42)
> On Tue, Mar 13, 2018 at 12:17:13AM +0000, Chris Wilson wrote:
> > Quoting Lucas De Marchi (2018-03-13 00:03:12)
> > > In some places we end up converting switch statements to a series of
> > > if/else, particularly when introducing helper functions to handle a
> > > group of cases. It's tempting to either leave a wrong warning (since now
> > > we don't have a switch case anymore) or to convert to WARN(1, ...),
> > > losing what MISSING_CASE() provides: source location and id number.
> > > 
> > > Fix the message to allow reusing MISSING_CASE() - it may not always be
> > > correct (e.g. if you are not checking an id anymore), but it avoids
> > > useless conversions. A quick grep reveals at least a few users in
> > > drivers/gpu/drm/i915/intel_csr.c and drivers/gpu/drm/i915/intel_ddi.c.
> > > 
> > > Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_utils.h | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h
> > > index 51dbfe5bb418..8cdc21b92f5f 100644
> > > --- a/drivers/gpu/drm/i915/i915_utils.h
> > > +++ b/drivers/gpu/drm/i915/i915_utils.h
> > > @@ -40,7 +40,7 @@
> > >  #undef WARN_ON_ONCE
> > >  #define WARN_ON_ONCE(x) WARN_ONCE((x), "%s", "WARN_ON_ONCE(" __stringify(x) ")")
> > >  
> > > -#define MISSING_CASE(x) WARN(1, "Missing switch case (%lu) in %s\n", \
> > > +#define MISSING_CASE(x) WARN(1, "Missing case (%lu) in %s\n", \
> > >                              (long)(x), __func__)
> > 
> > Whilst here you could make this more informative by:
> > "Missing case (%s = %lu) in %s\n", __stringify(x), (long)(x), __func__
> 
> The backtrace isn't enough?

Not if we have more than one in a function. (Why would we do that, you
might ask, and I'd answer if the point is to make this more generic and
versatile, then do so. We already have the value, why not then explain
what that value is.) And give me a single sentence identifying the missing
case makes it much more pleasant.
-Chris


More information about the Intel-gfx mailing list