Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
MIME-Version: 1.0
References: <20220311013238.3387227-1-pobrn@protonmail.com>
 <20220311013238.3387227-2-pobrn@protonmail.com> <CAHk-=wjkqz42CNjDgWA9U3uNWa9GriqaCqqKciqm0sZUYjfLQg@mail.gmail.com>
 <CAHk-=wjF4d_v5t=ht_vCOHxNDoPvsBuK-2jiEBus-__oPZuPFQ@mail.gmail.com> <wKlkWvCGvBrBjshT6gHT23JY9kWImhFPmTKfZWtN5Bkv_OtIFHTy7thr5SAEL6sYDthMDth-rvFETX-gCZPPCb9t2bO1zilj0Q-OTTSbe00=@protonmail.com>
In-Reply-To: <wKlkWvCGvBrBjshT6gHT23JY9kWImhFPmTKfZWtN5Bkv_OtIFHTy7thr5SAEL6sYDthMDth-rvFETX-gCZPPCb9t2bO1zilj0Q-OTTSbe00=@protonmail.com>
From:   Linus Torvalds <torvalds@linux-foundation.org>
Date:   Thu, 10 Mar 2022 21:06:59 -0800
Message-ID: <CAHk-=wg6PtRx9nBHVnrQkQkczGjXjO0E-oHXPPgPK_H+Nm=Czg@mail.gmail.com>
Subject: Re: [RFC PATCH v1 1/2] list: add type-safer list_head wrapper
To:     =?UTF-8?B?QmFybmFiw6FzIFDFkWN6ZQ==?= <pobrn@protonmail.com>
Cc:     Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Xiaomeng Tong <xiam0nd.tong@gmail.com>,
        Kees Cook <keescook@chromium.org>,
        Jakob Koschel <jakobkoschel@gmail.com>,
        Arnd Bergmann <arnd@arndb.de>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Thu, Mar 10, 2022 at 6:49 PM Barnab=C3=A1s P=C5=91cze <pobrn@protonmail.=
com> wrote:
>
> That is indeed a simpler thing to do, and doesn't need `offsetof()` at th=
e
> declaration, but there are places - not many -  where the `list_head` mem=
ber
> is inside a subobject, for example, so `member` now contains a period.

Ahh, very true. And very annoying. So close, yet so far, and no way I
can see to really deal with that.

And it's not even really all that rare. It may not be the _common_
case, but it's still fairly wide-spread and not some "one or two odd
places" thing.

This grep catches at least a subset of cases:

    git grep '\<list_for_each_entry(.*\.[a-z_0-9]*)'

and it's clearly all over.

As mentioned, I don't think that we have had huge problems with
getting the member name wrong. We do get a fair amount of checking in
that it obviously has to be part of the type we iterate over, and even
if you were to pick the wrong one, the result is a very simple "that
doesn't work".

But it would still undeniably be very nice to have some automatic
build-time checking for it.

Now, some checking could be done by just doing the "reverse" of what
that patch in

  https://lore.kernel.org/all/CAHk-=3DwiacQM76xec=3DHr7cLchVZ8Mo9VDHmXRJzJ_=
EX4sOsApEA@mail.gmail.com/

does with 'list_traversal_head()', and have a

  #define list_traversal_entry()

that has a similar union with a type that also specifies the list
entry type (with that same "append marker to member name" model, which
still works fine with dots in the middle).

Then at least cross-checking that the type of the iterator matches
with the target list member is trivial, but _if_ a type has multiple
list entries, they often end up referring to the same type.

That very patch with 'struct task_struct' converted is actually an
example of exactly that: the <children,sibling> and <ptraced,
ptrace_entry> pairings have the exact same type tuple (all being
'struct task_struct', of course).

So it would strengthen the typechecking a bit in some cases, but
probably not all that noticeably.

            Linus