On 18 September 2013 14:22, David Long <[email protected]> wrote: > On 09/18/13 05:21, Ryan Harkin wrote: > >> On 17 September 2013 01:58, David Long <[email protected]> wrote: >> >> From: "David A. Long" <[email protected]> >>> >>> Freeing a NULL pointer does not really have to be considered a fatal >>> error. >>> >>> Signed-off-by: David A. Long <[email protected]> >>> --- >>> 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 mailing list [email protected] http://lists.linaro.org/mailman/listinfo/boot-architecture
