Hello Jaben,
OK, thanks for explaining.
Thanks,
Scott
From: Carsey, Jaben [mailto:[email protected]]
Sent: Wednesday, September 24, 2014 03:47 PM
To: Scott Duplichan; [email protected]; Qiu, Shumin
Cc: Carsey, Jaben
Subject: RE: [edk2] [Patch] ShellPkg: Refine code style to avoid potential
NullPointer dereference.
The return status of the function is checked.
The ASSERT verifies that if the code ever changes such that a NULL pointer is
returned then debug builds of the code may notice
(depending on PCDs as you indicate). In release mode it will have zero impact.
The ASSERT is here to make sure that a developer doing a good unit test does
not accidentally change the code to have a valid return
value with a NULL pointer.
I believe that ASSERT is not valid for testing conditions that may occur based
on standard behavior (memory allocation failure or
user input), but are intended to be used to verify items that only a developer
can affect.
Note: the above belief does not always mean ASSERT is used that way, but it's
how I believe they should be used.
-Jaben
From: Scott Duplichan [mailto:[email protected]]
Sent: Wednesday, September 24, 2014 12:11 PM
To: [email protected] <mailto:[email protected]>
; Qiu, Shumin; Carsey, Jaben
Subject: RE: [edk2] [Patch] ShellPkg: Refine code style to avoid potential
NullPointer dereference.
Importance: High
Hello Jaben, Shumin or someone else,
Please help me understand this one line patch. How can the original code
dereference a null pointer? For both Mv.c and Cp.c, the
code that calls ShellLevel2StripQuotes() checks the return status before
attempting to dereference the pointer returned by the
function. If ShellLevel2StripQuotes() returns error, Mv.c and Cp.c both return
an error to their caller _before_ any attempt to
dereference the returned pointer. The function ShellLevel2StripQuotes() returns
an error status for all cases where it returns a
null pointer. What am I missing?
Also, is ASSERT guaranteed to stop execution for all build types and all values
of PcdDebugPropertyMask? If so, is stopping
execution a suitable error handling response for release code?
Thanks,
Scott
From: Carsey, Jaben [mailto:[email protected]]
Sent: Wednesday, September 24, 2014 12:12 PM
To: Qiu, Shumin
Cc: [email protected] <mailto:[email protected]>
Subject: Re: [edk2] [Patch] ShellPkg: Refine code style to avoid potential
NullPointer dereference.
Reviewed-by: Jaben Carsey <[email protected]
<mailto:[email protected]> >
From: Qiu, Shumin
Sent: Monday, September 22, 2014 8:03 PM
To: Carsey, Jaben
Cc: [email protected] <mailto:[email protected]>
Subject: [edk2] [Patch] ShellPkg: Refine code style to avoid potential
NullPointer dereference.
Importance: High
Hi Jaben,
Could you help review the patch? Pointer 'CleanFilePathStr' returned from
function 'ShellLevel2StripQuotes' may be NULL and may be
dereferenced.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qiu Shumin <[email protected] <mailto:[email protected]> >
Thanks,
Shumin
------------------------------------------------------------------------------
Meet PCI DSS 3.0 Compliance Requirements with EventLog Analyzer
Achieve PCI DSS 3.0 Compliant Status with Out-of-the-box PCI DSS Reports
Are you Audit-Ready for PCI DSS 3.0 Compliance? Download White paper
Comply to PCI DSS 3.0 Requirement 10 and 11.5 with EventLog Analyzer
http://pubads.g.doubleclick.net/gampad/clk?id=154622311&iu=/4140/ostg.clktrk
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-devel
