[0/2] xen/list: some fixes in list.h

Message

Juergen Gross Feb. 16, 2025, 10:23 a.m. UTC

Patch 1 is a fix for an undefined behavior reported by Andrew. I think
this patch should be considered for 4.20.

Patch 2 is fixing wrong comments in list.h I stumbled over when doing
patch 1. As it is absolutely no risk involved with this patch, I think
it should be 4.20 material, too.

There are some additional cleanups possible in list.h, which I can do
for 4.21 when wanted:

- Removal of list_prepare_entry(), which seems to be unused since
  some time now (and it seems to be thought of as a list.h internal
  macro only).

- More questionable: removal of unused iterators, like e.g.
  list_for_each_entry_continue() or list_for_each_entry_from(). The main
  idea to keep list.h in sync with the Linux version would be violated
  by this removal, though. OTOH with patch 1 they are out of sync anyway
  now, but I'm planning to submit a Linux kernel patch fixing the UB in
  the Linux variant, too.

Juergen Gross (2):
  xen/list: avoid UB in list iterators
  xen/list: fix comments in include/xen/list.h

 xen/include/xen/list.h | 144 +++++++++++++++++++++++++----------------
 1 file changed, 89 insertions(+), 55 deletions(-)

Comments

Oleksii Kurochko Feb. 17, 2025, 9:05 a.m. UTC | #1

On 2/16/25 11:23 AM, Juergen Gross wrote:
> Patch 1 is a fix for an undefined behavior reported by Andrew. I think
> this patch should be considered for 4.20.
>
> Patch 2 is fixing wrong comments in list.h I stumbled over when doing
> patch 1. As it is absolutely no risk involved with this patch, I think
> it should be 4.20 material, too.
>
> There are some additional cleanups possible in list.h, which I can do
> for 4.21 when wanted:
>
> - Removal of list_prepare_entry(), which seems to be unused since
>    some time now (and it seems to be thought of as a list.h internal
>    macro only).
>
> - More questionable: removal of unused iterators, like e.g.
>    list_for_each_entry_continue() or list_for_each_entry_from(). The main
>    idea to keep list.h in sync with the Linux version would be violated
>    by this removal, though. OTOH with patch 1 they are out of sync anyway
>    now, but I'm planning to submit a Linux kernel patch fixing the UB in
>    the Linux variant, too.
>
> Juergen Gross (2):
>    xen/list: avoid UB in list iterators
>    xen/list: fix comments in include/xen/list.h

With getting of proper Acks we can have both patches in 4.20:
  Release-Acked-by: Oleksii Kurochko<[e-mail]>

Thanks.

~ Oleksii

>
>   xen/include/xen/list.h | 144 +++++++++++++++++++++++++----------------
>   1 file changed, 89 insertions(+), 55 deletions(-)
>

Jan Beulich Feb. 17, 2025, 9:52 a.m. UTC | #2

On 16.02.2025 11:23, Juergen Gross wrote:
> Patch 1 is a fix for an undefined behavior reported by Andrew. I think
> this patch should be considered for 4.20.
> 
> Patch 2 is fixing wrong comments in list.h I stumbled over when doing
> patch 1. As it is absolutely no risk involved with this patch, I think
> it should be 4.20 material, too.
> 
> There are some additional cleanups possible in list.h, which I can do
> for 4.21 when wanted:
> 
> - Removal of list_prepare_entry(), which seems to be unused since
>   some time now (and it seems to be thought of as a list.h internal
>   macro only).
> 
> - More questionable: removal of unused iterators, like e.g.
>   list_for_each_entry_continue() or list_for_each_entry_from(). The main
>   idea to keep list.h in sync with the Linux version would be violated
>   by this removal, though.

That's true for the unused list_prepare_entry(), too, isn't it? Which in
turn is coupled with list_for_each_entry_continue().

> OTOH with patch 1 they are out of sync anyway
>   now, but I'm planning to submit a Linux kernel patch fixing the UB in
>   the Linux variant, too.

I'd go with whatever the Linux side is going accept.

Jan