[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aCXQ2mdVU_D4tFiS@gmail.com>
Date: Thu, 15 May 2025 13:32:42 +0200
From: Ingo Molnar <mingo@...nel.org>
To: Andy Shevchenko <andy@...nel.org>
Cc: linux-kernel@...r.kernel.org, Arnd Bergmann <arnd@...nel.org>,
Borislav Petkov <bp@...en8.de>, Juergen Gross <jgross@...e.com>,
"H . Peter Anvin" <hpa@...or.com>,
Kees Cook <keescook@...omium.org>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Mike Rapoport <rppt@...nel.org>,
Paul Menzel <pmenzel@...gen.mpg.de>,
Peter Zijlstra <peterz@...radead.org>,
Thomas Gleixner <tglx@...utronix.de>,
David Woodhouse <dwmw@...zon.co.uk>
Subject: [PATCH 31/29] x86/boot/e820: Move index increments outside accessors
in e820__update_table()
* Andy Shevchenko <andy@...nel.org> wrote:
> On Mon, Apr 21, 2025 at 08:51:57PM +0200, Ingo Molnar wrote:
>
> ...
>
> > + int idx;
>
> > + u32 idx, chg_idx, chg_nr;
>
> What about sanitizing the type as well to be let's say unsigned int idx in all cases?
Not sure - I kind of like the brevity of 'u32' here, and it's also the
type of the originating ABI interface.
> ...
>
> > + change_point[chg_idx]->addr = entries[idx].addr;
> > + change_point[chg_idx++]->entry = &entries[idx];
> > + change_point[chg_idx]->addr = entries[idx].addr + entries[idx].size;
> > + change_point[chg_idx++]->entry = &entries[idx];
>
> Does GCC 15 not produce any warnings here? Linus recently complain on some code
> with index increment inside the accessor. Perhaps just
>
> change_point[chg_idx]->entry = &entries[idx];
> chg_idx++;
>
> ?
Yeah, good point - patch attached.
Thanks,
Ingo
===================================>
From: Ingo Molnar <mingo@...nel.org>
Date: Thu, 15 May 2025 13:26:46 +0200
Subject: [PATCH] x86/boot/e820: Move index increments outside accessors in e820__update_table()
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@...nel.org>
Signed-off-by: Ingo Molnar <mingo@...nel.org>
Cc: Arnd Bergmann <arnd@...nel.org>
Cc: David Woodhouse <dwmw@...zon.co.uk>
Cc: H. Peter Anvin <hpa@...or.com>
Cc: Kees Cook <keescook@...omium.org>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Mike Rapoport (Microsoft) <rppt@...nel.org>
---
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 aadc46f3d074..b758c0632d0c 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) {
change_point[chg_idx]->addr = entries[idx].addr;
- change_point[chg_idx++]->entry = &entries[idx];
+ change_point[chg_idx]->entry = &entries[idx];
+ chg_idx++;
change_point[chg_idx]->addr = entries[idx].addr + entries[idx].size;
- change_point[chg_idx++]->entry = &entries[idx];
+ change_point[chg_idx]->entry = &entries[idx];
+ chg_idx++;
}
}
chg_nr = chg_idx;
Powered by blists - more mailing lists