Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <c0fc6e9c096778dce5c1e63c29af5ebdce83aca6.camel@HansenPartnership.com>
Subject: Re: [PATCH 2/6] list: add new MACROs to make iterator invisiable
 outside the loop
From:   James Bottomley <James.Bottomley@HansenPartnership.com>
To:     Xiaomeng Tong <xiam0nd.tong@gmail.com>,
        torvalds@linux-foundation.org
Cc:     arnd@arndb.de, jakobkoschel@gmail.com,
        linux-kernel@vger.kernel.org, gregkh@linuxfoundation.org,
        keescook@chromium.org, jannh@google.com,
        linux-kbuild@vger.kernel.org, linux-mm@kvack.org,
        netdev@vger.kernel.org
Date:   Wed, 02 Mar 2022 08:02:23 -0500
In-Reply-To: <20220301075839.4156-3-xiam0nd.tong@gmail.com>
References: <20220301075839.4156-1-xiam0nd.tong@gmail.com>
         <20220301075839.4156-3-xiam0nd.tong@gmail.com>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.34.4 
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Tue, 2022-03-01 at 15:58 +0800, Xiaomeng Tong wrote:
> For each list_for_each_entry* macros(10 variants), implements a
> respective
> new *_inside one. Such as the new macro list_for_each_entry_inside
> for
> list_for_each_entry. The idea is to be as compatible with the
> original
> interface as possible and to minimize code changes.
> 
> Here are 2 examples:
> 
> list_for_each_entry_inside:
>  - declare the iterator-variable pos inside the loop. Thus, the
> origin
>    declare of the inputed *pos* outside the loop should be removed.
> In
>    other words, the inputed *pos* now is just a string name.
>  - add a new "type" argument as the type of the container struct this
> is
>    embedded in, and should be inputed when calling the macro.
> 
> list_for_each_entry_safe_continue_inside:
>  - declare the iterator-variable pos and n inside the loop. Thus, the
>    origin declares of the inputed *pos* and *n* outside the loop
> should
>    be removed. In other words, the inputed *pos* and *n* now are just
>    string name.
>  - add a new "start" argument as the given iterator to start with and
>    can be used to get the container struct *type*. This should be
> inputed
>    when calling the macro.
> 
> Signed-off-by: Xiaomeng Tong <xiam0nd.tong@gmail.com>
> ---
>  include/linux/list.h | 156
> +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 156 insertions(+)
> 
> diff --git a/include/linux/list.h b/include/linux/list.h
> index dd6c2041d..1595ce865 100644
> --- a/include/linux/list.h
> +++ b/include/linux/list.h
> @@ -639,6 +639,19 @@ static inline void list_splice_tail_init(struct
> list_head *list,
>  	     !list_entry_is_head(pos, head, member);			
> \
>  	     pos = list_next_entry(pos, member))
>  
> +/**
> + * list_for_each_entry_inside
> + *  - iterate over list of given type and keep iterator inside the
> loop
> + * @pos:	the type * to use as a loop cursor.
> + * @type:	the type of the container struct this is embedded in.
> + * @head:	the head for your list.
> + * @member:	the name of the list_head within the struct.
> + */
> +#define list_for_each_entry_inside(pos, type, head, member)		
> \
> +	for (type * pos = list_first_entry(head, type, member);		
> \
> +	     !list_entry_is_head(pos, head, member);			
> \
> +	     pos = list_next_entry(pos, member))
> +
>  

pos shouldn't be an input to the macro since it's being declared inside
it.  All that will do will set up confusion about the shadowing of pos.
The macro should still work as

#define list_for_each_entry_inside(type, head, member) \
  ...

For safety, you could

#define POS __UNIQUE_ID(pos)

and use POS as the loop variable .. you'll have to go through an
intermediate macro to get it to be stable.  There are examples in
linux/rcupdate.h

James