> On Dec 3, 2014, at 3:16 PM, Carsey, Jaben <jaben.car...@intel.com> wrote:
>
> If the compiler guarantees a valid pointer, but the code checker cant know
> that, I think that they are trying to get around the code checker, to reduce
> false positives, not fix the code.
>
But why use an ASSERT for that? If MDEPKG_NDEBUG is defined the “Fix does not
work!” as Expression is never evaluated. Thus for some release configurations
the code checker is still going to fail! This is not a tool chain portable fix.
https://svn.code.sf.net/p/edk2/code/trunk/edk2/MdePkg/Include/Library/DebugLib.h
#if !defined(MDEPKG_NDEBUG)
#define ASSERT(Expression) \
do { \
if (DebugAssertEnabled ()) { \
if (!(Expression)) { \
_ASSERT (Expression); \
} \
} \
} while (FALSE)
#else
#define ASSERT(Expression)
#endif
ASSERT’s exist to help debug code, not to handle errors in code or work around
a code checker.
Thanks,
Andrew Fish
> From: Andrew Fish [mailto:af...@apple.com]
> Sent: Wednesday, December 03, 2014 2:56 PM
> To: Carsey, Jaben
> Cc: edk2-devel@lists.sourceforge.net
> Subject: Re: [edk2] [Patch] ShellPkg: Add ASSERT to check pointer to avoid
> being dereferenced
> Importance: High
>
>
> On Dec 3, 2014, at 12:00 PM, Carsey, Jaben <jaben.car...@intel.com
> <mailto:jaben.car...@intel.com>> wrote:
>
> So is the ASSERT to get past an automated code checking tool warning?
>
>
> Which is why I’m confused, as it still seems like a subset of release builds
> would fail? Don’t you want to check the code you release to customers?
>
> Thanks,
>
> Andrew Fish
>
>
>
>
> From: Qiu, Shumin [mailto:shumin....@intel.com <mailto:shumin....@intel.com>]
> Sent: Wednesday, December 03, 2014 4:44 AM
> To: edk2-devel@lists.sourceforge.net
> <mailto:edk2-devel@lists.sourceforge.net>; Andrew Fish
> Subject: Re: [edk2] [Patch] ShellPkg: Add ASSERT to check pointer to avoid
> being dereferenced
>
> Hi Andrew,
> ASSERT is here to make sure developer will not change code to return a NULL
> pointer. In release it has no impact as HiiGetString can always return proper
> string back or the build will fail.
>
> -Shumin
>
> From: Andrew Fish [mailto:af...@apple.com <mailto:af...@apple.com>]
> Sent: Wednesday, December 03, 2014 11:09 AM
> To: edk2-devel@lists.sourceforge.net <mailto:edk2-devel@lists.sourceforge.net>
> Subject: Re: [edk2] [Patch] ShellPkg: Add ASSERT to check pointer to avoid
> being dereferenced
>
>
> On Dec 2, 2014, at 6:56 PM, Qiu, Shumin <shumin....@intel.com
> <mailto:shumin....@intel.com>> wrote:
>
> Hi Jaben,
> Could you help review the patch? This patch adds ‘ASSERT’ to check the
> pointer returned from HiiGetString to avoid pointer dereferenced.
>
>
> Why are we adding ASSERT to check pointers. It is likely turned off in
> release builds. ASSERTs are a debugging aid, not a robustness check.
>
> The shell is an application that runs on a large number of system so it
> should handle the errors in C code, and it could also ASSERT() on a debug
> build.
>
> Thanks,
>
> Andrew Fish
>
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Qiu Shumin <shumin....@intel.com <mailto:shumin....@intel.com>>
>
> Thanks
> Shumin
> <UefiHandleParsingLib.c.patch>------------------------------------------------------------------------------
> Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
> from Actuate! Instantly Supercharge Your Business Reports and Dashboards
> with Interactivity, Sharing, Native Excel Exports, App Integration & more
> Get technology previously reserved for billion-dollar corporations, FREE
> http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk_______________________________________________
>
> <http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk_______________________________________________>
> edk2-devel mailing list
> edk2-devel@lists.sourceforge.net <mailto:edk2-devel@lists.sourceforge.net>
> https://lists.sourceforge.net/lists/listinfo/edk2-devel
> <https://lists.sourceforge.net/lists/listinfo/edk2-devel>
------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel
