linux-kernel - Re: [PATCH] kernel.h: Add for_each

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]

Date:   Fri, 13 Jul 2018 16:42:59 -0700
From:   Randy Dunlap <rdunlap@...radead.org>
To:     NeilBrown <neilb@...e.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Daniel Vetter <daniel@...ll.ch>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        DRI Development <dri-devel@...ts.freedesktop.org>,
        Intel Graphics Development <intel-gfx@...ts.freedesktop.org>,
        Gustavo Padovan <gustavo@...ovan.org>,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Sean Paul <seanpaul@...omium.org>,
        David Airlie <airlied@...ux.ie>,
        Kees Cook <keescook@...omium.org>,
        Ingo Molnar <mingo@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Wei Wang <wvw@...gle.com>, Stefan Agner <stefan@...er.ch>,
        Andrei Vagin <avagin@...nvz.org>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Yisheng Xie <ysxie@...mail.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Daniel Vetter <daniel.vetter@...el.com>
Subject: Re: [PATCH] kernel.h: Add for_each_if()

On 07/13/2018 04:37 PM, NeilBrown wrote:
> On Wed, Jul 11 2018, Andrew Morton wrote:
> 
>> On Wed, 11 Jul 2018 13:51:08 +0200 Daniel Vetter <daniel@...ll.ch> wrote:
>>
>>> But I still have the situation that a bunch of maintainers acked this
>>> and Andrew Morton defacto nacked it, which I guess means I'll keep the
>>> macro in drm? The common way to go about this seems to be to just push
>>> the patch series with the ack in some pull request to Linus and ignore
>>> the people who raised questions, but not really my thing.
>>
>> Heh.
>>
>> But, am I wrong?  Code which uses regular kernel style doesn't have
>> these issues.  We shouldn't be enabling irregular style - we should be
>> making such sites more regular.  The fact that the compiler generates a
>> nice warning in some cases simply helps us with that.
> 
> I think you are wrong .... or at least, not completely correct.
> 
> I think it is perfectly acceptable in Linux to have code like:
> 
>   for (....)
>   	if (x)
>         	something();
>         else
>         	something_else();
> 
> Would you agree?  If not, then I'm the one who is wrong.  Otherwise....

coding-style.rst says:
Also, use braces when a loop contains more than a single simple statement:


> The problem is that for certain poorly written for_each_foo() macros,
> such as blkg_for_each_descendant_pre() (and several others identified in
> this patch series), writing
> 
>    blkg_for_each_descendant_pre(...)
>      	if (x)
>         	something();
>         else
>         	something_else();
> 
> will trigger a compiler warning.  This is inconsistent with the
> behaviour of a simple "for".
> So I do think that the macros should be fixed, and I don't think that
> sprinkling extra braces is an appropriate response.
> 
> I'm not personally convinced that writing
>    if_no_else(cond)
> is easier than just writing
>    if (!(cond)); else

agreed.

> in these macros, but I do think that the macros should be fixed and
> maybe this is the path-of-least-resistance to getting it done.

I'm not opposed to fixing some macros, but some of these macros are just
ease-of-less-typing shortcuts.  They don't improve readability at all;
they harm it.  (of course, that is just one opinion :)



-- 
~Randy