| Message ID | 20240730111132.1097315-3-sughosh.ganu@linaro.org |
|---|---|
| State | Accepted |
| Commit | 7aa0addc42ef1674a7baaf9d0428db943c3f2e5f |
| Headers | show |
| Series | [1/5] linux: list: add a function to count list nodes | expand |
On Tue, 30 Jul 2024 at 05:11, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > Populate the previous memory descriptor node pointer only after it's its > parent struct has been initialised. The compiler fixes this logic to > do the right thing, but it is better to have correct code in place. > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > --- > lib/efi_loader/efi_memory.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) Reviewed-by: Simon Glass <sjg@chromium.org> > > diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c > index bfadd6bd41..8d4f6e339d 100644 > --- a/lib/efi_loader/efi_memory.c > +++ b/lib/efi_loader/efi_memory.c > @@ -138,7 +138,7 @@ static void efi_mem_sort(void) > merge_again = false; > list_for_each(lhandle, &efi_mem) { > struct efi_mem_list *lmem; > - struct efi_mem_desc *prev = &prevmem->desc; > + struct efi_mem_desc *prev; > struct efi_mem_desc *cur; > uint64_t pages; > > @@ -149,6 +149,7 @@ static void efi_mem_sort(void) > } > > cur = &lmem->desc; > + prev = &prevmem->desc; > > if ((desc_get_end(cur) == prev->physical_start) && > (prev->type == cur->type) && > -- > 2.34.1 >
On 7/30/24 13:11, Sughosh Ganu wrote: > Populate the previous memory descriptor node pointer only after it's > parent struct has been initialised. The compiler fixes this logic to > do the right thing, but it is better to have correct code in place. > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> Fixes: 7b05667ce239 ("efi_loader: Merge memory map entries") Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > --- > lib/efi_loader/efi_memory.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c > index bfadd6bd41..8d4f6e339d 100644 > --- a/lib/efi_loader/efi_memory.c > +++ b/lib/efi_loader/efi_memory.c > @@ -138,7 +138,7 @@ static void efi_mem_sort(void) > merge_again = false; > list_for_each(lhandle, &efi_mem) { > struct efi_mem_list *lmem; > - struct efi_mem_desc *prev = &prevmem->desc; > + struct efi_mem_desc *prev; > struct efi_mem_desc *cur; > uint64_t pages; > > @@ -149,6 +149,7 @@ static void efi_mem_sort(void) > } > > cur = &lmem->desc; > + prev = &prevmem->desc; > > if ((desc_get_end(cur) == prev->physical_start) && > (prev->type == cur->type) &&
On Wed, 31 Jul 2024 at 09:19, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > On 7/30/24 13:11, Sughosh Ganu wrote: > > Populate the previous memory descriptor node pointer only after it's > > parent struct has been initialised. The compiler fixes this logic to > > do the right thing, but it is better to have correct code in place. > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > > Fixes: 7b05667ce239 ("efi_loader: Merge memory map entries") > Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > > > --- > > lib/efi_loader/efi_memory.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c > > index bfadd6bd41..8d4f6e339d 100644 > > --- a/lib/efi_loader/efi_memory.c > > +++ b/lib/efi_loader/efi_memory.c > > @@ -138,7 +138,7 @@ static void efi_mem_sort(void) > > merge_again = false; > > list_for_each(lhandle, &efi_mem) { > > struct efi_mem_list *lmem; > > - struct efi_mem_desc *prev = &prevmem->desc; > > + struct efi_mem_desc *prev; > > struct efi_mem_desc *cur; > > uint64_t pages; > > > > @@ -149,6 +149,7 @@ static void efi_mem_sort(void) > > } > > > > cur = &lmem->desc; > > + prev = &prevmem->desc; > > > > if ((desc_get_end(cur) == prev->physical_start) && > > (prev->type == cur->type) && > Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c index bfadd6bd41..8d4f6e339d 100644 --- a/lib/efi_loader/efi_memory.c +++ b/lib/efi_loader/efi_memory.c @@ -138,7 +138,7 @@ static void efi_mem_sort(void) merge_again = false; list_for_each(lhandle, &efi_mem) { struct efi_mem_list *lmem; - struct efi_mem_desc *prev = &prevmem->desc; + struct efi_mem_desc *prev; struct efi_mem_desc *cur; uint64_t pages; @@ -149,6 +149,7 @@ static void efi_mem_sort(void) } cur = &lmem->desc; + prev = &prevmem->desc; if ((desc_get_end(cur) == prev->physical_start) && (prev->type == cur->type) &&
Populate the previous memory descriptor node pointer only after it's parent struct has been initialised. The compiler fixes this logic to do the right thing, but it is better to have correct code in place. Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> --- lib/efi_loader/efi_memory.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)