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.lo...@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.g...@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;
  }

  /**


_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to