linux-kernel - Re: [Intel-gfx] [PATCH RESEND] drm/i915: Fix pipe/transcoder enum mismatches

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANEJEGs0WARN-w4Gm9O39uB53BfV35VXWgUibsCX3UYpMBNxfQ@mail.gmail.com>
Date:   Fri, 5 May 2017 13:29:32 -0700
From:   Grant Grundler <grundler@...omium.org>
To:     Ville Syrjälä <ville.syrjala@...ux.intel.com>
Cc:     Grant Grundler <grundler@...omium.org>,
        Matthias Kaehlcke <mka@...omium.org>,
        Daniel Vetter <daniel.vetter@...el.com>,
        Jani Nikula <jani.nikula@...ux.intel.com>,
        David Airlie <airlied@...ux.ie>,
        intel-gfx@...ts.freedesktop.org,
        LKML <linux-kernel@...r.kernel.org>,
        Greg Hackmann <ghackmann@...gle.com>,
        Michael Davidson <md@...gle.com>,
        dri-devel@...ts.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH RESEND] drm/i915: Fix pipe/transcoder enum mismatches

On Fri, May 5, 2017 at 1:08 PM, Ville Syrjälä
<ville.syrjala@...ux.intel.com> wrote:
...
>> > I'm not convinced the patch is making things any better really. To
>> > fix this really properly, I think we'd need to introduce a new enum
>> > pch_transcoder and thus avoid the confusion of which type of
>> > transcoder we're talking about.
>>
>> Is an enum better than coding an explicit conversion in an inline function?
>
> The point of the enum would be to make it more clear which piece of
> hardware we're talking to in each case.

Ah ok - I misunderstood - I thought this was already the case.

> But this would require going
> through the entire PCH code and changing things to use the right type
> in each case. Quite a bit of work with little measurable gain I'd say.

IMHO, one of the best things that happened to C standard was addition
of strong type checking. It's helped prevent developers from making
one class of "stupid mistakes". So while this change wouldn't improve
performance, it would allow a form of automated correctness checking
that can be enforced with every patch you get (every time the code
base is compiled).

In other words, the gain isn't currently measurable the same way
performance is but I believe it's worth doing.  Given the number of
typedefs and enums in kernel code, I suspect most kernel developers
would agree.

cheers,
grant



>
>> Then the code can do some sanity checking as well. Something like:
>>
>> enum transcoder pch_to_cpu_enum(enum pipe)
>> {
>>     WARN_ON(pipe > FOO);
>>     return (enum transcoder) pipe;
>> }
>
> That would have to be called pipe_to_pch_transcoder() or something like
> that.
>
>>
>> > Currently most places expect an
>> > enum pipe when dealing with PCH transcoders, and enum transcoder
>> > when dealing with CPU transcoders. But there are some exceptions
>> > of course.
>>
>> cheers,
>> grant
>> >
>> > --
>> > Ville Syrjälä
>> > Intel OTC
>
> --
> Ville Syrjälä
> Intel OTC