| Message ID | 1456395736-11784-1-git-send-email-lersek@redhat.com |
|---|---|
| State | Accepted |
| Commit | ffbb5ae3ba7da2ece8dbf116b1eb0718c346d19b |
| Headers | show |
Cinnamon, can you please R-b this version as well? It is trivial, I just wanted to be real clean in this fix. Thanks Laszlo On 02/25/16 11:22, Laszlo Ersek wrote: > 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 <cecil.sheng@hpe.com> > Cc: Cinnamon Shia <cinnamon.shia@hpe.com> > Cc: Eric Dong <eric.dong@intel.com> > Cc: Qiu Shumin <shumin.qiu@intel.com> > Cc: Samer El-Haj-Mahmoud <elhaj@hpe.com> > Cc: Yao Jiewen <Jiewen.Yao@intel.com> > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Laszlo Ersek <lersek@redhat.com> > --- > > Notes: > v2: > - Drop incorrect explicit (VOID *) cast in assignment to EvalOnce. > - Build-tested only. > > v1: > - 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..ca478de68e77 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 = (p); \ > + 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 edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 02/26/16 17:43, Shia, Cinnamon wrote: > Hi Laszlo, > > Sorry for the late response. > If the code to do type casting is removed, is EvalOnce still needed? The type of the input parameter of FreePool is (VOID *) the same as EvalOnce. > > Something like: > > #define free(p) \ > do { \ > if (p != NULL) { \ > FreePool (p); \ > } \ > } while (FALSE) > > Thanks, > Cinnamon Shia Yes, EvalOnce is needed -- it's primary purpose is to prevent double evaluation between the comparison of "p" against NULL, and the passing of "p" to FreePool(). The (VOID *) cast was unrelated to EvalOnce -- EvalOnce was just the right place to do the cast, as long as I assumed the cast made any sense. Consider a use case like this: /* Free all elements in the array. Some of them may be NULL. */ i = 0; while (i < count) { free(array_of_pointers[i++]); } This would break if you removed EvalOnce, because for non-NULL elements, "i" would be incremented twice, and the non-NULL check and the FreePool() call would refer to different elements. (Side note: since free() is a standard C function, I used a coding style in the above example that is closer to standard C than to edk2.) Thanks Laszlo > > -----Original Message----- > From: Laszlo Ersek [mailto:lersek@redhat.com] > Sent: Friday, February 26, 2016 11:03 PM > To: Shia, Cinnamon > Cc: edk2-devel-01; Eric Dong; Sheng, Cecil (HPS SW); Qiu Shumin; Yao Jiewen; El-Haj-Mahmoud, Samer > Subject: Re: [edk2] [PATCH v2] MdeModulePkg: RegularExpressionDxe: support free(NULL) > > Cinnamon, > > can you please R-b this version as well? It is trivial, I just wanted to be real clean in this fix. > > Thanks > Laszlo > > On 02/25/16 11:22, Laszlo Ersek wrote: >> 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 <cecil.sheng@hpe.com> >> Cc: Cinnamon Shia <cinnamon.shia@hpe.com> >> Cc: Eric Dong <eric.dong@intel.com> >> Cc: Qiu Shumin <shumin.qiu@intel.com> >> Cc: Samer El-Haj-Mahmoud <elhaj@hpe.com> >> Cc: Yao Jiewen <Jiewen.Yao@intel.com> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Laszlo Ersek <lersek@redhat.com> >> --- >> >> Notes: >> v2: >> - Drop incorrect explicit (VOID *) cast in assignment to EvalOnce. >> - Build-tested only. >> >> v1: >> - Build-tested only. >> >> >> MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/OnigurumaUefiPor >> t.h | 12 +++++++++++- >> 1 file changed, 11 insertions(+), 1 deletion(-) >> >> diff --git >> a/MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/OnigurumaUefiP >> ort.h >> b/MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/OnigurumaUefiP >> ort.h index cb791f8c84c6..ca478de68e77 100644 >> --- >> a/MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/OnigurumaUefiP >> ort.h >> +++ b/MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/OnigurumaU >> +++ efiPort.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 = (p); \ >> + 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 edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 02/26/16 18:16, Shia, Cinnamon wrote: > Hi Lazlo and David, > > Thanks for your explanation. :) > > Reviewed-By: Cinnamon Shia <cinnamon.shia@hpe.com> Thank you, pushed as ffbb5ae3ba7da2ece8dbf116b1eb0718c346d19b. Laszlo > > Thanks, > Cinnamon Shia > > -----Original Message----- > From: Laszlo Ersek [mailto:lersek@redhat.com] > Sent: Saturday, February 27, 2016 1:00 AM > To: Shia, Cinnamon > Cc: edk2-devel-01; Eric Dong; Sheng, Cecil (HPS SW); Qiu Shumin; Yao Jiewen; El-Haj-Mahmoud, Samer > Subject: Re: [edk2] [PATCH v2] MdeModulePkg: RegularExpressionDxe: support free(NULL) > > On 02/26/16 17:43, Shia, Cinnamon wrote: >> Hi Laszlo, >> >> Sorry for the late response. >> If the code to do type casting is removed, is EvalOnce still needed? The type of the input parameter of FreePool is (VOID *) the same as EvalOnce. >> >> Something like: >> >> #define free(p) \ >> do { \ >> if (p != NULL) { \ >> FreePool (p); \ >> } \ >> } while (FALSE) >> >> Thanks, >> Cinnamon Shia > > Yes, EvalOnce is needed -- it's primary purpose is to prevent double evaluation between the comparison of "p" against NULL, and the passing of "p" to FreePool(). The (VOID *) cast was unrelated to EvalOnce -- EvalOnce was just the right place to do the cast, as long as I assumed the cast made any sense. > > Consider a use case like this: > > /* Free all elements in the array. Some of them may be NULL. */ > i = 0; > while (i < count) { > free(array_of_pointers[i++]); > } > > This would break if you removed EvalOnce, because for non-NULL elements, "i" would be incremented twice, and the non-NULL check and the > FreePool() call would refer to different elements. > > (Side note: since free() is a standard C function, I used a coding style in the above example that is closer to standard C than to edk2.) > > Thanks > Laszlo > > >> >> -----Original Message----- >> From: Laszlo Ersek [mailto:lersek@redhat.com] >> Sent: Friday, February 26, 2016 11:03 PM >> To: Shia, Cinnamon >> Cc: edk2-devel-01; Eric Dong; Sheng, Cecil (HPS SW); Qiu Shumin; Yao >> Jiewen; El-Haj-Mahmoud, Samer >> Subject: Re: [edk2] [PATCH v2] MdeModulePkg: RegularExpressionDxe: >> support free(NULL) >> >> Cinnamon, >> >> can you please R-b this version as well? It is trivial, I just wanted to be real clean in this fix. >> >> Thanks >> Laszlo >> >> On 02/25/16 11:22, Laszlo Ersek wrote: >>> 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 <cecil.sheng@hpe.com> >>> Cc: Cinnamon Shia <cinnamon.shia@hpe.com> >>> Cc: Eric Dong <eric.dong@intel.com> >>> Cc: Qiu Shumin <shumin.qiu@intel.com> >>> Cc: Samer El-Haj-Mahmoud <elhaj@hpe.com> >>> Cc: Yao Jiewen <Jiewen.Yao@intel.com> >>> Contributed-under: TianoCore Contribution Agreement 1.0 >>> Signed-off-by: Laszlo Ersek <lersek@redhat.com> >>> --- >>> >>> Notes: >>> v2: >>> - Drop incorrect explicit (VOID *) cast in assignment to EvalOnce. >>> - Build-tested only. >>> >>> v1: >>> - Build-tested only. >>> >>> >>> MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/OnigurumaUefiPo >>> r >>> t.h | 12 +++++++++++- >>> 1 file changed, 11 insertions(+), 1 deletion(-) >>> >>> diff --git >>> a/MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/OnigurumaUefi >>> P >>> ort.h >>> b/MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/OnigurumaUefi >>> P ort.h index cb791f8c84c6..ca478de68e77 100644 >>> --- >>> a/MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/OnigurumaUefi >>> P >>> ort.h >>> +++ b/MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/Oniguruma >>> +++ U >>> +++ efiPort.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 = (p); \ >>> + 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 edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Hi Laszlo, On 25 February 2016 at 10:22, Laszlo Ersek <lersek@redhat.com> wrote: > 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 <cecil.sheng@hpe.com> > Cc: Cinnamon Shia <cinnamon.shia@hpe.com> > Cc: Eric Dong <eric.dong@intel.com> > Cc: Qiu Shumin <shumin.qiu@intel.com> > Cc: Samer El-Haj-Mahmoud <elhaj@hpe.com> > Cc: Yao Jiewen <Jiewen.Yao@intel.com> > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Laszlo Ersek <lersek@redhat.com> > --- > > Notes: > v2: > - Drop incorrect explicit (VOID *) cast in assignment to EvalOnce. > - Build-tested only. > > v1: > - 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..ca478de68e77 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 = (p); \ > + if (EvalOnce != NULL) { \ > + FreePool (EvalOnce); \ > + } \ > + } while (FALSE) > + Just for me education, can you explain what the do{...}while(FALSE) loop is for? Thanks, Ryan. > #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) > -- > 1.8.3.1 > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 02/26/16 19:48, Ryan Harkin wrote: > Hi Laszlo, > > On 25 February 2016 at 10:22, Laszlo Ersek <lersek@redhat.com> wrote: >> 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 <cecil.sheng@hpe.com> >> Cc: Cinnamon Shia <cinnamon.shia@hpe.com> >> Cc: Eric Dong <eric.dong@intel.com> >> Cc: Qiu Shumin <shumin.qiu@intel.com> >> Cc: Samer El-Haj-Mahmoud <elhaj@hpe.com> >> Cc: Yao Jiewen <Jiewen.Yao@intel.com> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Laszlo Ersek <lersek@redhat.com> >> --- >> >> Notes: >> v2: >> - Drop incorrect explicit (VOID *) cast in assignment to EvalOnce. >> - Build-tested only. >> >> v1: >> - 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..ca478de68e77 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 = (p); \ >> + if (EvalOnce != NULL) { \ >> + FreePool (EvalOnce); \ >> + } \ >> + } while (FALSE) >> + > > Just for me education, can you explain what the do{...}while(FALSE) loop is for? Sure. It is for supporting the semicolon after free(x) in all contexts. Consider the case when you don't have the do / while on the outside, just the brackets. The following would break: if (blah) free(x); else something_else(); A semicolon right after a bracketed block is a harmless null statement in most contexts, but not in the one above. Thanks Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 26 February 2016 at 19:20, Laszlo Ersek <lersek@redhat.com> wrote: > On 02/26/16 19:48, Ryan Harkin wrote: >> Hi Laszlo, >> >> On 25 February 2016 at 10:22, Laszlo Ersek <lersek@redhat.com> wrote: >>> 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 <cecil.sheng@hpe.com> >>> Cc: Cinnamon Shia <cinnamon.shia@hpe.com> >>> Cc: Eric Dong <eric.dong@intel.com> >>> Cc: Qiu Shumin <shumin.qiu@intel.com> >>> Cc: Samer El-Haj-Mahmoud <elhaj@hpe.com> >>> Cc: Yao Jiewen <Jiewen.Yao@intel.com> >>> Contributed-under: TianoCore Contribution Agreement 1.0 >>> Signed-off-by: Laszlo Ersek <lersek@redhat.com> >>> --- >>> >>> Notes: >>> v2: >>> - Drop incorrect explicit (VOID *) cast in assignment to EvalOnce. >>> - Build-tested only. >>> >>> v1: >>> - 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..ca478de68e77 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 = (p); \ >>> + if (EvalOnce != NULL) { \ >>> + FreePool (EvalOnce); \ >>> + } \ >>> + } while (FALSE) >>> + >> >> Just for me education, can you explain what the do{...}while(FALSE) loop is for? > > Sure. It is for supporting the semicolon after free(x) in all contexts. > > Consider the case when you don't have the do / while on the outside, > just the brackets. The following would break: > > if (blah) > free(x); > else > something_else(); > > A semicolon right after a bracketed block is a harmless null statement > in most contexts, but not in the one above. > There you go. That's something I once knew and forgot. I'm sure I used to do it a different way than do...while, but that's by-the-by. It's been almost 20 years since I was in macro hell :-) Thanks! _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 02/26/16 20:35, Ryan Harkin wrote: > On 26 February 2016 at 19:20, Laszlo Ersek <lersek@redhat.com> wrote: >> On 02/26/16 19:48, Ryan Harkin wrote: >>> Hi Laszlo, >>> >>> On 25 February 2016 at 10:22, Laszlo Ersek <lersek@redhat.com> wrote: >>>> 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 <cecil.sheng@hpe.com> >>>> Cc: Cinnamon Shia <cinnamon.shia@hpe.com> >>>> Cc: Eric Dong <eric.dong@intel.com> >>>> Cc: Qiu Shumin <shumin.qiu@intel.com> >>>> Cc: Samer El-Haj-Mahmoud <elhaj@hpe.com> >>>> Cc: Yao Jiewen <Jiewen.Yao@intel.com> >>>> Contributed-under: TianoCore Contribution Agreement 1.0 >>>> Signed-off-by: Laszlo Ersek <lersek@redhat.com> >>>> --- >>>> >>>> Notes: >>>> v2: >>>> - Drop incorrect explicit (VOID *) cast in assignment to EvalOnce. >>>> - Build-tested only. >>>> >>>> v1: >>>> - 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..ca478de68e77 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 = (p); \ >>>> + if (EvalOnce != NULL) { \ >>>> + FreePool (EvalOnce); \ >>>> + } \ >>>> + } while (FALSE) >>>> + >>> >>> Just for me education, can you explain what the do{...}while(FALSE) loop is for? >> >> Sure. It is for supporting the semicolon after free(x) in all contexts. >> >> Consider the case when you don't have the do / while on the outside, >> just the brackets. The following would break: >> >> if (blah) >> free(x); >> else >> something_else(); >> >> A semicolon right after a bracketed block is a harmless null statement >> in most contexts, but not in the one above. >> > > There you go. That's something I once knew and forgot. I'm sure I > used to do it a different way than do...while, but that's by-the-by. > > It's been almost 20 years since I was in macro hell :-) Yeah, normally this kind of thing gets done with inline functions nowadays, I think. Disclaimer: please do not ask me how stringification (#) and token pasting (##) work in the preprocessor, especially when you use them in macros that are supposed to take other macros as arguments. That's something I've never been able to wrap my head around, for longer than 2 minutes. Also don't ask me about the representation of bit-fields. I read those passages in the C standard once; ever since I've been pretending that bit-fields don't exist (beyond recognizing them in the source and being extra careful about them). Cheers! Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
diff --git a/MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/OnigurumaUefiPort.h b/MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/OnigurumaUefiPort.h index cb791f8c84c6..ca478de68e77 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 = (p); \ + 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)
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 <cecil.sheng@hpe.com> Cc: Cinnamon Shia <cinnamon.shia@hpe.com> Cc: Eric Dong <eric.dong@intel.com> Cc: Qiu Shumin <shumin.qiu@intel.com> Cc: Samer El-Haj-Mahmoud <elhaj@hpe.com> Cc: Yao Jiewen <Jiewen.Yao@intel.com> Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- Notes: v2: - Drop incorrect explicit (VOID *) cast in assignment to EvalOnce. - Build-tested only. v1: - Build-tested only. MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/OnigurumaUefiPort.h | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) -- 1.8.3.1 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel