Re: [PATCH] drm/i915/ddi: Silence compiler warning for unknown output type

[Jani Nikula <jani.nikula@linux.intel.com>]

On Mon, 04 Apr 2016, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Mon, Apr 04, 2016 at 09:20:27AM +0300, Joonas Lahtinen wrote:
>> On su, 2016-04-03 at 21:59 +0100, Chris Wilson wrote:
>> > Silences
>> > 
>> > 	src/drivers/gpu/drm/i915/intel_ddi.c: warning: 'port' may be used uninitialized in this function [-Wuninitialized]
>> > 
>> > Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
>> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> > ---
>> >  drivers/gpu/drm/i915/intel_ddi.c | 6 +++---
>> >  1 file changed, 3 insertions(+), 3 deletions(-)
>> > 
>> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
>> > index 766156f88ef4..921edf183d22 100644
>> > --- a/drivers/gpu/drm/i915/intel_ddi.c
>> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
>> > @@ -315,6 +315,9 @@ static void ddi_get_encoder_port(struct intel_encoder *intel_encoder,
>> >  		*dig_port = enc_to_mst(encoder)->primary;
>> >  		*port = (*dig_port)->port;
>> >  		break;
>> > +	default:
>> > +		WARN(1, "Invalid DDI encoder type %d\n", intel_encoder->type);
>> > +		/* fallthrough and treat as unknown */
>> 
>> While touching it, might as well fix into MISSING_CASE. With that;
>
> I was tempted, I wasn't sure if not using MISSING_CASE was deliberate
> (trying not to confuse semantics between adding support for a new
> generation versus an unknown DDI)? So I left it for whoever has to make
> that fix (and kept the change as small as possible for a minor
> buglet).

Hitting MISSING_CASE here would, IMO, imply that the fix is to add the
missing case, but we have plenty of intel_output_type enums that simply
should not be handled here at all, and we should ensure we don't end up
here with the wrong output types. Thus I think keeping the WARN is the
way to go.

I'm just slightly concerned about the fix here though. So we're screwed
anyway if the output type is not handled, and we might use an
uninitialized variable... but the assumption that using *dig_port =
enc_to_dig_port(encoder) will result in any more meaningful result is
wrong.

BR,
Jani.


>  
>> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>
> Thanks,
> -Chris

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx