On 16.07.2024 18:12, Elias El Yandouzi wrote: > Hi Jan, > > On 14/05/2024 16:03, Jan Beulich wrote: >> On 13.05.2024 15:40, Elias El Yandouzi wrote: >>> --- a/xen/arch/x86/pv/dom0_build.c >>> +++ b/xen/arch/x86/pv/dom0_build.c >>> @@ -382,6 +382,10 @@ int __init dom0_construct_pv(struct domain *d, >>> l3_pgentry_t *l3tab = NULL, *l3start = NULL; >>> l2_pgentry_t *l2tab = NULL, *l2start = NULL; >>> l1_pgentry_t *l1tab = NULL, *l1start = NULL; >>> + mfn_t l4start_mfn = INVALID_MFN; >>> + mfn_t l3start_mfn = INVALID_MFN; >>> + mfn_t l2start_mfn = INVALID_MFN; >>> + mfn_t l1start_mfn = INVALID_MFN; >> >> Just to mention it here again: By limiting the scope of these I'm pretty >> sure no initializer would be needed even "just in case" (really I don't >> think they're needed even when the all have function scope, as producer >> and consumer are always close together afaics; quite different from >> l<N>start and l<N>tab). > > I had a closer look at your suggestion and I don't think it is possible, > especially for l3start_mfn. > > The variable, l3start_mfn, can get initialized in the else leg of the > first if statement along with l3start variable. > > If you look a few lines below in the for loop, we call > l4e_from_mfn(l3start_mfn, L4_PROT) which assumes l3start_mfn is valid. > It could not be the case if we took the first leg of the aforementioned > if statement. > > I don't think I can this easily limit their scope. It could work for the > others, but not l3start_mfn. So I can either leave things as they are or > limit the scope of every variables but l3start_mfn.
Please do. Limiting the scope of variables, especially in larger functions, is often quite helpful. The one exception is certainly necessary here, I agree. Jan
