On 02/25/16 09:56, Shia, Cinnamon wrote: > Reviewed-By: Cinnamon Shia <[email protected]>
Thanks, but actually I noticed an error in this patch: > -----Original Message----- > From: Laszlo Ersek [mailto:[email protected]] > Sent: Thursday, February 25, 2016 5:14 AM > To: edk2-devel-01 > Cc: Sheng, Cecil (HPS SW); Shia, Cinnamon; Eric Dong; Qiu Shumin; > El-Haj-Mahmoud, Samer; Yao Jiewen > Subject: [PATCH 4/4] MdeModulePkg: RegularExpressionDxe: support free(NULL) > > The ISO C standard says about free(), > > If ptr is a null pointer, no action occurs. > > This is not true of the FreePool() interface of the MemoryAllocationLib > class: > > Buffer must have been allocated on a previous call to the pool > allocation services of the Memory Allocation Library. [...] If Buffer > was not allocated with a pool allocation function in the Memory > Allocation Library, then ASSERT(). > > Therefore we must not forward the argument of free() to FreePool() without > checking. > > Cc: Cecil Sheng <[email protected]> > Cc: Cinnamon Shia <[email protected]> > Cc: Eric Dong <[email protected]> > Cc: Qiu Shumin <[email protected]> > Cc: Samer El-Haj-Mahmoud <[email protected]> > Cc: Yao Jiewen <[email protected]> > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Laszlo Ersek <[email protected]> > --- > > Notes: > Build-tested only. > > MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/OnigurumaUefiPort.h | > 12 +++++++++++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git > a/MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/OnigurumaUefiPort.h > b/MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/OnigurumaUefiPort.h > index cb791f8c84c6..b970aaa48c0e 100644 > --- > a/MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/OnigurumaUefiPort.h > +++ > b/MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/OnigurumaUefiPort.h > @@ -30,7 +30,17 @@ typedef UINTN size_t; > > #define malloc(n) AllocatePool(n) > #define calloc(n,s) AllocateZeroPool((n)*(s)) > -#define free(p) FreePool(p) > + > +#define free(p) \ > + do { \ > + VOID *EvalOnce; \ > + \ > + EvalOnce = (VOID *)(p); \ the explicit cast to (VOID *) is an error here. I think I was simply too tired when I wrote this patch last night. The explicit (VOID *) cast will *incorrectly* suppress errors that a direct call to FreePool() or free() would report. Consider the following example: STATIC CONST CHAR8 MyString[] = "Hello World"; CONST VOID *MyPointer; MyPointer = MyString; // valid FreePool (MyPointer); // invalid, but caught by the compiler! The pre-patch macro definition will catch this -- the compiler will report a warning (usually treated as an error) that the pointer conversion (implicit in passing the argument to FreePool(), which takes a (VOID *)) throws away the CONST qualifier. Whereas the modified macro definition will silently suppress that. So, I will go ahead and commit patches 1-3 in this series, with Qin Long's R-b; but I will submit a new version of this patch separately. Thanks, and sorry for the churn. Laszlo > + if (EvalOnce != NULL) { \ > + FreePool (EvalOnce); \ > + } \ > + } while (FALSE) > + > #define realloc(OldPtr,NewSize,OldSize) > ReallocatePool(OldSize,NewSize,OldPtr) > #define xmemmove(Dest,Src,Length) CopyMem(Dest,Src,Length) > #define xmemcpy(Dest,Src,Length) CopyMem(Dest,Src,Length) > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
