On Sep 16, 2013, at 11:36 PM, Ryan Harkin <ryan.har...@linaro.org> wrote:
> Hi,
>
> Tracking down a crash I am seeing shows that I am effectively calling
> FreePool() with a NULL pointer. The system then ASSERTS and hangs.
>
> I've noticed various seemingly "random" asserts like this before and suspect
> it may be related. Of course, the assert output never helps track down the
> culprit, but that's a different issue altogether.
>
> Coming from an environment where I'm able to call malloc() and then free()
> with a NULL pointer, my experience told me that I should be able to call
> FreePool() with a NULL pointer without worrying about checking the pointer
> for NULL first.
>
> However, CoreFreePool() that eventually gets called is explicitly returning
> EFI_INVALID_PARAMETER if it is called with NULL. And FreePool() explicitly
> ASSERTs if the return value from CoreFreePool() is not EFI_SUCCESS.
>
> It doesn't strike me as a sensible thing to do.
>
> I think FreePool(NULL) should just return without reporting an error. I see
> a *lot* of calls to FreePool where the pointer isn't first checked for NULL.
> The code path is often simpler, where the code will often return if the alloc
> fails, but in my example, I am retrieving an optional variable which may or
> may not exist and therefore may or may not allocate memory.
>
gBS->FreePool() returns an error, per the spec, as it should.
> Although systems seems to have got along just fine up to now - the code has
> always been like this - I'd like to change it. Hanging seems unnecessarily
> harsh.
>
> What do others think?
>
> Is there a real advantage to hanging on NULL?
Technically the system is not hung, it is likely sitting at a breakpoint or a
dead loop, so you can step past in a debugger. You also get a nice stack trace
with a debugger.
There are lots of knobs to control this behavior. You can turn off ASSERTs, and
you can also make an ASSERT, breakpoint, dead-loop, or continue automatically.
https://svn.code.sf.net/p/edk2/code/trunk/edk2/MdePkg/Include/Library/DebugLib.h
/**
Prints an assert message containing a filename, line number, and description.
This may be followed by a breakpoint or a dead loop.
Print a message of the form "ASSERT <FileName>(<LineNumber>): <Description>\n"
to the debug output device. If DEBUG_PROPERTY_ASSERT_BREAKPOINT_ENABLED bit
of
PcdDebugProperyMask is set then CpuBreakpoint() is called. Otherwise, if
DEBUG_PROPERTY_ASSERT_DEADLOOP_ENABLED bit of PcdDebugProperyMask is set then
CpuDeadLoop() is called. If neither of these bits are set, then this
function
returns immediately after the message is printed to the debug output device.
DebugAssert() must actively prevent recursion. If DebugAssert() is called
while
processing another DebugAssert(), then DebugAssert() must return immediately.
If FileName is NULL, then a <FileName> string of "(NULL) Filename" is printed.
If Description is NULL, then a <Description> string of "(NULL) Description"
is printed.
@param FileName The pointer to the name of the source file that
generated the assert condition.
@param LineNumber The line number in the source file that generated the
assert condition
@param Description The pointer to the description of the assert condition.
**/
VOID
EFIAPI
DebugAssert (
IN CONST CHAR8 *FileName,
IN UINTN LineNumber,
IN CONST CHAR8 *Description
);
/**
Internal worker macro that calls DebugAssert().
This macro calls DebugAssert(), passing in the filename, line number, and an
expression that evaluated to FALSE.
@param Expression Boolean expression that evaluated to FALSE
**/
#define _ASSERT(Expression) DebugAssert (__FILE__, __LINE__, #Expression)
#if !defined(MDEPKG_NDEBUG)
#define ASSERT(Expression) \
do { \
if (DebugAssertEnabled ()) { \
if (!(Expression)) { \
_ASSERT (Expression); \
} \
} \
} while (FALSE)
#else
#define ASSERT(Expression)
#endif
>
> Regards,
> Ryan.
> ------------------------------------------------------------------------------
> LIMITED TIME SALE - Full Year of Microsoft Training For Just $49.99!
> 1,500+ hours of tutorials including VisualStudio 2012, Windows 8, SharePoint
> 2013, SQL 2012, MVC 4, more. BEST VALUE: New Multi-Library Power Pack includes
> Mobile, Cloud, Java, and UX Design. Lowest price ever! Ends 9/20/13.
> http://pubads.g.doubleclick.net/gampad/clk?id=58041151&iu=/4140/ostg.clktrk_______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/edk2-devel
------------------------------------------------------------------------------
LIMITED TIME SALE - Full Year of Microsoft Training For Just $49.99!
1,500+ hours of tutorials including VisualStudio 2012, Windows 8, SharePoint
2013, SQL 2012, MVC 4, more. BEST VALUE: New Multi-Library Power Pack includes
Mobile, Cloud, Java, and UX Design. Lowest price ever! Ends 9/20/13.
http://pubads.g.doubleclick.net/gampad/clk?id=58041151&iu=/4140/ostg.clktrk
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel
