| Message ID | 20190714230108.25764-7-pure.logic@nexus-software.ie |
|---|---|
| State | Superseded, archived |
| Delegated to: | Heinrich Schuchardt |
| Headers | show |
| Series | efi_loader: Fix inconsistencies in efi_add_memory_map usage | expand |
On 7/15/19 1:01 AM, Bryan O'Donoghue wrote: > efi_add_memory_map() now returns efi_status_t not the passed uint64_t > address on success. We need to capture that change in efi_free_pages(). > > Signed-off-by: Bryan O'Donoghue <pure.logic@nexus-software.ie> > --- > lib/efi_loader/efi_memory.c | 11 ++++------- > 1 file changed, 4 insertions(+), 7 deletions(-) > > diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c > index 2b06acb2ae..9c59a9637c 100644 > --- a/lib/efi_loader/efi_memory.c > +++ b/lib/efi_loader/efi_memory.c > @@ -496,7 +496,6 @@ void *efi_alloc(uint64_t len, int memory_type) > */ > efi_status_t efi_free_pages(uint64_t memory, efi_uintn_t pages) > { > - uint64_t r = 0; > efi_status_t ret; > > ret = efi_check_allocated(memory, true); > @@ -510,13 +509,11 @@ efi_status_t efi_free_pages(uint64_t memory, efi_uintn_t pages) > return EFI_INVALID_PARAMETER; > } > > - r = efi_add_memory_map(memory, pages, EFI_CONVENTIONAL_MEMORY, false); > - /* Merging of adjacent free regions is missing */ Please, leave this TODO comment in the code. It is really something that is worth fixing. As said the patches of the series should be squashed. E.g. with 'rebase -i HEAD~8'. When bisecting an unrelated problem you do not want to end up with something not working which is only the result of changes belonging together being in separate patches. Patch 2 with the function description is the only one that in principle could be separated. But I see no need to do so. Otherwise the series is ok. Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > - > - if (r == memory) > - return EFI_SUCCESS; > + ret = efi_add_memory_map(memory, pages, EFI_CONVENTIONAL_MEMORY, false); > + if (ret != EFI_SUCCESS) > + return EFI_NOT_FOUND; > > - return EFI_NOT_FOUND; > + return ret; > } > > /** >
On 15/07/2019 00:57, Heinrich Schuchardt wrote:
> Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
NP I'll squash these down in the morning.
diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c index 2b06acb2ae..9c59a9637c 100644 --- a/lib/efi_loader/efi_memory.c +++ b/lib/efi_loader/efi_memory.c @@ -496,7 +496,6 @@ void *efi_alloc(uint64_t len, int memory_type) */ efi_status_t efi_free_pages(uint64_t memory, efi_uintn_t pages) { - uint64_t r = 0; efi_status_t ret; ret = efi_check_allocated(memory, true); @@ -510,13 +509,11 @@ efi_status_t efi_free_pages(uint64_t memory, efi_uintn_t pages) return EFI_INVALID_PARAMETER; } - r = efi_add_memory_map(memory, pages, EFI_CONVENTIONAL_MEMORY, false); - /* Merging of adjacent free regions is missing */ - - if (r == memory) - return EFI_SUCCESS; + ret = efi_add_memory_map(memory, pages, EFI_CONVENTIONAL_MEMORY, false); + if (ret != EFI_SUCCESS) + return EFI_NOT_FOUND; - return EFI_NOT_FOUND; + return ret; } /**
efi_add_memory_map() now returns efi_status_t not the passed uint64_t address on success. We need to capture that change in efi_free_pages(). Signed-off-by: Bryan O'Donoghue <pure.logic@nexus-software.ie> --- lib/efi_loader/efi_memory.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)