From: "David A. Long" dave.long@linaro.org
Freeing a NULL pointer does not really have to be considered a fatal error.
Signed-off-by: David A. Long dave.long@linaro.org --- MdePkg/Library/UefiMemoryAllocationLib/MemoryAllocationLib.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/MdePkg/Library/UefiMemoryAllocationLib/MemoryAllocationLib.c b/MdePkg/Library/UefiMemoryAllocationLib/MemoryAllocationLib.c index 0ccb524..3333bc7 100644 --- a/MdePkg/Library/UefiMemoryAllocationLib/MemoryAllocationLib.c +++ b/MdePkg/Library/UefiMemoryAllocationLib/MemoryAllocationLib.c @@ -811,7 +811,10 @@ FreePool ( { EFI_STATUS Status;
- Status = gBS->FreePool (Buffer); - ASSERT_EFI_ERROR (Status); + if (Buffer == NULL) { + DEBUG((EFI_D_WARN, "FreePool: attempt to free NULL pointer\n")); + } else { + Status = gBS->FreePool (Buffer); + ASSERT_EFI_ERROR (Status); + } } -
On 17 September 2013 01:58, David Long dave.long@linaro.org wrote:
From: "David A. Long" dave.long@linaro.org
Freeing a NULL pointer does not really have to be considered a fatal error.
Signed-off-by: David A. Long dave.long@linaro.org
MdePkg/Library/UefiMemoryAllocationLib/MemoryAllocationLib.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/MdePkg/Library/UefiMemoryAllocationLib/MemoryAllocationLib.c b/MdePkg/Library/UefiMemoryAllocationLib/MemoryAllocationLib.c index 0ccb524..3333bc7 100644 --- a/MdePkg/Library/UefiMemoryAllocationLib/MemoryAllocationLib.c +++ b/MdePkg/Library/UefiMemoryAllocationLib/MemoryAllocationLib.c @@ -811,7 +811,10 @@ FreePool ( { EFI_STATUS Status;
- Status = gBS->FreePool (Buffer);
- ASSERT_EFI_ERROR (Status);
- if (Buffer == NULL) {
- DEBUG((EFI_D_WARN, "FreePool: attempt to free NULL pointer\n"));
- } else {
- Status = gBS->FreePool (Buffer);
- ASSERT_EFI_ERROR (Status);
- }
}
-- 1.8.1.2
Funny enough, I found this same problem yesterday too. I started a discussion about it on the edk2 mailing list. It seems people are in favour of the status-quo as usual, despite the ASSERT being pointless.
I'm not sure if we should carry such a patch if upstream will never accept it. Adding a wrapper function that doesn't assert might be the best option.
boot-architecture mailing list boot-architecture@lists.linaro.org http://lists.linaro.org/mailman/listinfo/boot-architecture
On 09/18/13 05:21, Ryan Harkin wrote:
On 17 September 2013 01:58, David Long dave.long@linaro.org wrote:
From: "David A. Long" dave.long@linaro.org
Freeing a NULL pointer does not really have to be considered a fatal error.
Signed-off-by: David A. Long dave.long@linaro.org
MdePkg/Library/UefiMemoryAllocationLib/MemoryAllocationLib.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/MdePkg/Library/UefiMemoryAllocationLib/MemoryAllocationLib.c b/MdePkg/Library/UefiMemoryAllocationLib/MemoryAllocationLib.c index 0ccb524..3333bc7 100644 --- a/MdePkg/Library/UefiMemoryAllocationLib/MemoryAllocationLib.c +++ b/MdePkg/Library/UefiMemoryAllocationLib/MemoryAllocationLib.c @@ -811,7 +811,10 @@ FreePool ( { EFI_STATUS Status;
- Status = gBS->FreePool (Buffer);
- ASSERT_EFI_ERROR (Status);
- if (Buffer == NULL) {
- DEBUG((EFI_D_WARN, "FreePool: attempt to free NULL pointer\n"));
- } else {
- Status = gBS->FreePool (Buffer);
- ASSERT_EFI_ERROR (Status);
- } }
-- 1.8.1.2
Funny enough, I found this same problem yesterday too. I started a discussion about it on the edk2 mailing list. It seems people are in favour of the status-quo as usual, despite the ASSERT being pointless.
I'm not sure if we should carry such a patch if upstream will never accept it. Adding a wrapper function that doesn't assert might be the best option.
I actually had sent this patch to an engineer at AMD a few days earlier. I should have sent it to the list then and saved you some trouble.
The patch did feel maybe a little controversial, but I didn't relish tracking down where the call was being made from. Is there any backtrace ability for debugging UEFI?
-dl
On 18 September 2013 14:22, David Long dave.long@linaro.org wrote:
On 09/18/13 05:21, Ryan Harkin wrote:
On 17 September 2013 01:58, David Long dave.long@linaro.org wrote:
From: "David A. Long" dave.long@linaro.org
Freeing a NULL pointer does not really have to be considered a fatal error.
Signed-off-by: David A. Long dave.long@linaro.org
MdePkg/Library/**UefiMemoryAllocationLib/**MemoryAllocationLib.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/MdePkg/Library/**UefiMemoryAllocationLib/** MemoryAllocationLib.c b/MdePkg/Library/**UefiMemoryAllocationLib/**MemoryAllocationLib.c index 0ccb524..3333bc7 100644 --- a/MdePkg/Library/**UefiMemoryAllocationLib/**MemoryAllocationLib.c +++ b/MdePkg/Library/**UefiMemoryAllocationLib/**MemoryAllocationLib.c @@ -811,7 +811,10 @@ FreePool ( { EFI_STATUS Status;
- Status = gBS->FreePool (Buffer);
- ASSERT_EFI_ERROR (Status);
- if (Buffer == NULL) {
- DEBUG((EFI_D_WARN, "FreePool: attempt to free NULL pointer\n"));
- } else {
- Status = gBS->FreePool (Buffer);
- ASSERT_EFI_ERROR (Status);
- } }
-- 1.8.1.2
Funny enough, I found this same problem yesterday too. I started a
discussion about it on the edk2 mailing list. It seems people are in favour of the status-quo as usual, despite the ASSERT being pointless.
I'm not sure if we should carry such a patch if upstream will never accept it. Adding a wrapper function that doesn't assert might be the best option.
I actually had sent this patch to an engineer at AMD a few days earlier. I should have sent it to the list then and saved you some trouble.
That's fine, it took me some debugging just to work out that I was indeed passing a null pointer in anyway.
The patch did feel maybe a little controversial,
The replies on the list vary in their support for the idea so far, but I haven't actually seen any serious objection, other than we would have to get it past the maintainer and change the spec.
So perhaps it isn't as far out there after all.
but I didn't relish tracking down where the call was being made from. Is there any backtrace ability for debugging UEFI?
Theoretically. But I'm the worst person to ask about debugging UEFI. I have a JTAG debugger I can use, but it's often more hassle that it's worth.
-dl
boot-architecture@lists.linaro.org