Hi Andrew,
You are right. I will update the patch.
-Shumin
From: Andrew Fish [mailto:af...@apple.com]
Sent: Thursday, December 04, 2014 7:28 AM
To: Carsey, Jaben
Cc: edk2-devel@lists.sourceforge.net
Subject: Re: [edk2] [Patch] ShellPkg: Add ASSERT to check pointer to avoid
being dereferenced
On Dec 3, 2014, at 3:16 PM, Carsey, Jaben
<jaben.car...@intel.com<mailto: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<mailto: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]
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]
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_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net>
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
