Re: [PATCH 32/32] x86/boot/e820: Move index increments outside accessors in e820__update_table()

[Nikolay Borisov]

On 5/15/25 15:05, Ingo Molnar wrote:
> This kind of code:
>
> 	change_point[chg_idx++]->entry  = &entries[idx];
>
> Can be a bit confusing to human readers, and GCC-15 started
> warning about these patterns.
>
> Move the index increment outside the accessor.
>
> Suggested-by: Andy Shevchenko <andy@xxxxxxxxxx>
> Signed-off-by: Ingo Molnar <mingo@xxxxxxxxxx>
> Cc: Arnd Bergmann <arnd@xxxxxxxxxx>
> Cc: David Woodhouse <dwmw@xxxxxxxxxxxx>
> Cc: H. Peter Anvin <hpa@xxxxxxxxx>
> Cc: Kees Cook <keescook@xxxxxxxxxxxx>
> Cc: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
> Cc: Mike Rapoport (Microsoft) <rppt@xxxxxxxxxx>
> ---
>   arch/x86/kernel/e820.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
> index 10c6e7dc72d7..afb312620c82 100644
> --- a/arch/x86/kernel/e820.c
> +++ b/arch/x86/kernel/e820.c
> @@ -421,9 +421,11 @@ __init int e820__update_table(struct e820_table *table)
>   	for (idx = 0; idx < table->nr_entries; idx++)	{
>   		if (entries[idx].size != 0) {
nit: The level of nesting can easily be reduced by doing
if (entries[idx].size == 0)
	continue;
>   			change_point[chg_idx]->addr	= entries[idx].addr;
> -			change_point[chg_idx++]->entry	= &entries[idx];
> +			change_point[chg_idx]->entry	= &entries[idx];
> +			chg_idx++;

nit: I have to agree with H. Peter Anvin that this seems somewhat odd to 
me as well.

<snip>