linux-kernel - Re: [PATCH v3] drm/i915: remove IS

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] [day] [month] [year] [list]

Date:   Fri, 8 Oct 2021 00:49:44 +0900
From:   Masahiro Yamada <masahiroy@...nel.org>
To:     Lucas De Marchi <lucas.demarchi@...el.com>
Cc:     intel-gfx <intel-gfx@...ts.freedesktop.org>,
        dri-devel <dri-devel@...ts.freedesktop.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Dan Carpenter <dan.carpenter@...cle.com>,
        Steven Price <steven.price@....com>,
        Chris Wilson <chris@...is-wilson.co.uk>,
        Andrzej Hajda <a.hajda@...sung.com>,
        Jani Nikula <jani.nikula@...el.com>
Subject: Re: [PATCH v3] drm/i915: remove IS_ACTIVE

On Wed, Oct 6, 2021 at 2:21 AM Lucas De Marchi <lucas.demarchi@...el.com> wrote:
>
> When trying to bring IS_ACTIVE to linux/kconfig.h I thought it wouldn't
> provide much value just encapsulating it in a boolean context. So I also
> added the support for handling undefined macros as the IS_ENABLED()
> counterpart. However the feedback received from Masahiro Yamada was that
> it is too ugly, not providing much value. And just wrapping in a boolean
> context is too dumb - we could simply open code it.
>
> As detailed in commit babaab2f4738 ("drm/i915: Encapsulate kconfig
> constant values inside boolean predicates"), the IS_ACTIVE macro was
> added to workaround a compilation warning. However after checking again
> our current uses of IS_ACTIVE it turned out there is only
> 1 case in which it triggers a warning in clang (due
> -Wconstant-logical-operand) and 2 in smatch. All the others
> can simply use the shorter version, without wrapping it in any macro.
>
> So here I'm dialing all the way back to simply removing the macro. That
> single case hit by clang can be changed to make the constant come first,
> so it doesn't think it's mask:
>
>         -       if (context && CONFIG_DRM_I915_FENCE_TIMEOUT)
>         +       if (CONFIG_DRM_I915_FENCE_TIMEOUT && context)
>
> As talked with Dan Carpenter, that logic will be added in smatch as
> well, so it will also stop warning about it.
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@...el.com>
> Acked-by: Jani Nikula <jani.nikula@...el.com>

Reviewed-by: Masahiro Yamada <masahiroy@...nel.org>





-- 
Best Regards
Masahiro Yamada