On 07.08.2023 17:03, Nicola Vetrini wrote: > On 07/08/2023 10:09, Jan Beulich wrote: >> On 04.08.2023 17:27, Nicola Vetrini wrote: >>> The variable declared in the header file >>> 'xen/arch/x86/include/asm/e820.h' >>> is shadowed by many function parameters, so it is renamed to avoid >>> these >>> violations. >>> >>> No functional changes. >>> >>> Signed-off-by: Nicola Vetrini <nicola.vetr...@bugseng.com> >>> --- >>> This patch is similar to other renames done on previous patches, and >>> the >>> preferred strategy there was to rename the global variable. This one >>> has more occurrences that are spread in various files, but >>> the general pattern is the same. >> >> Still I think it would be better done the other way around, and perhaps >> in >> more than a single patch. It looks like "many == 3", i.e. >> - e820_add_range(), which is only ever called with "e820" as its >> argument, >> and hence the parameter could be dropped, >> - e820_change_range_type(), which is in the same situation, and >> - reserve_e820_ram(), which wants its parameter renamed. > > This one is defined as > > return e820_change_range_type(e820, s, e, E820_RAM, E820_RESERVED); > > so I'm not certain that the parameter can be dropped from that function,
You're right, it can't. I didn't pay enough attention on the interaction of both. So renaming it is here then as well. Jan > because the cascade effect > would eliminate the need to have a 'boot_e820' in 'xen/arch/x86/setup.c' > afaict and since the comment says > > /* A temporary copy of the e820 map that we can mess with during > bootstrap. */ > static struct e820map __initdata boot_e820; > > I'm not sure it's a good idea to alter this call chain. >
