Date:   Sat, 5 Mar 2022 13:09:01 -0800
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Xiaomeng Tong <xiam0nd.tong@...il.com>
Cc:     Arnd Bergmann <arnd@...db.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jakob Koschel <jakobkoschel@...il.com>,
        Jann Horn <jannh@...gle.com>,
        Kees Cook <keescook@...omium.org>,
        Linux Kbuild mailing list <linux-kbuild@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux-MM <linux-mm@...ck.org>, Netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH 2/6] list: add new MACROs to make iterator invisiable
 outside the loop

On Thu, Mar 3, 2022 at 6:51 PM Xiaomeng Tong <xiam0nd.tong@...il.com> wrote:
>
> >  - it means that the already *good* cases are the ones that are
> > penalized by having to change
>
> Yes, but it also kills potential risks that one day somebody mistakely
> uses iterator after the loop in this already *good* cases, as it removed
> the original declare of pos and any use-after-loop will be catched by
> compiler.

The thing is, I think we already have a solution to that case.

I think it's the bad "entry used outside" that we need to care about doing well.

> 3. restore all name back to list_for_each_entry after everything is done:
>    (minus 2 chars)

You are ignoring the big elephant in the room - counting the small
things, but not counting the BIG thing.

That type name argument is long.

Right now we avoid it by pre-declaring it, and that's in many ways the
natural thing to do in C (you don't declare types in the place that
uses them, you declare the types in the variable declarations above
the code).

Now, I'd love for the list head entry itself to "declare the type",
and solve it that way. That would in many ways be the optimal
situation, in that when a structure has that

        struct list_head xyz;

entry, it would be lovely to declare *there* what the list entry type
is - and have 'list_for_each_entry()' just pick it up that way.

It would be doable in theory - with some preprocessor trickery all the
'struct list_head' things *could* be made to be unnamed unions of the
list head, and the actual type it points to, ie something like

   #define declare_list_head(type,type) union { struct list_head x;
type *x##_list_type; }

and then (to pick one particular example), we could make the "struct
task_struct" entry for children be

-       struct list_head                children;
+       declare_list_head(struct task_struct, children);

and now when you use

        list_for_each_entry(p, &father->children, sibling) {

you could actually pick out the type with some really ugly
preprocessor crud, by doing 'typeof(*head##_list_type)' to get the
type of the thing we iterate over.

So we *could* embed the type that a list head points to with tricks
like that. The it would actually be type-safe, and not need a
declaration of the type anywhere. And it would be kind of nice to
document "this is a list head pointer to this kind of type".

And yes, it would be even better if we could also encode the member
name that contains the list entries somehow (ie in this case the
'sibling' list entry of the task struct) so that you'd really document
the full chain. But even my twisted mind cannot come up with any
tricks to do *that*.

But the above would be quite a *major* change.

And the above kind of preprocessor trickery and encoding a secondary
type as a union entry that isn't actually used for anythign else may
be too ugly to live anyway.

                 Linus

Powered by blists - more mailing lists