Re: [edk2-devel] [edk2 Patch 1/1] EmbeddedPkg: compiler error due to arithmetic operation on void pointer

["Laszlo Ersek" <lersek@redhat.com>]

On 2/6/24 16:55, Jayaprakash, N wrote:
> Thank you Laszlo and Ard Biesheuvel for your inputs.
> 2 type cast operations are not needed. 
> Shall I send an updated patch for review and merger?

Yes, please send a new version. It's a build error fix with low
regression risk, so it should qualify for merging during the hard freeze
even, IMO.

BTW, do you have an idea why this issue has popped up only now? The code
comes from commit 0d39caefb95e ("EmbeddedPkg/PrePiMemoryAllocationLib:
Add ReallocatePool", 2024-01-03), so it's a month old. Is this perhaps
the first time that the new ReallocatePool() function has been built
with MSVC? (I suspect that because the compiler error message in BZ 4668
seems to come from MSVC.)

Laszlo

> 
> Regards,
> JP 
> 
> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com> 
> Sent: Tuesday, February 6, 2024 8:07 PM
> To: Ard Biesheuvel <ardb@kernel.org>; devel@edk2.groups.io; Jayaprakash, N <n.jayaprakash@intel.com>
> Cc: Rebecca Cran <rebecca@bsdio.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Leif Lindholm <quic_llindhol@quicinc.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>; Abner Chang <abner.chang@amd.com>
> Subject: Re: [edk2-devel] [edk2 Patch 1/1] EmbeddedPkg: compiler error due to arithmetic operation on void pointer
> 
> On 2/6/24 13:57, Ard Biesheuvel wrote:
>> On Tue, 6 Feb 2024 at 05:23, Jayaprakash, N <n.jayaprakash@intel.com> wrote:
>>>
>>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4668
>>>
>>> This commit fixes the issue reported in the BZ4668.
>>> The EmbeddedPkg fails to compile with a compiler error generated due 
>>> to invalid/illegal arithmetic operation on void pointers. It has been 
>>> fixed by using explicit type conversion of the void pointer to UINTN.
>>>
>>> Cc: Rebecca Cran <rebecca@bsdio.com>
>>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>>> Cc: Laszlo Ersek <lersek@redhat.com>
>>> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
>>> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
>>> Cc: Abner Chang <abner.chang@amd.com>
>>> Cc: Jayaprakash N <n.jayaprakash@intel.com>
>>> Signed-off-by: Jayaprakash N <n.jayaprakash@intel.com>
>>> ---
>>>  .../Library/PrePiMemoryAllocationLib/MemoryAllocationLib.c      | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git 
>>> a/EmbeddedPkg/Library/PrePiMemoryAllocationLib/MemoryAllocationLib.c 
>>> b/EmbeddedPkg/Library/PrePiMemoryAllocationLib/MemoryAllocationLib.c
>>> index fa81cc9d59..11566cf57f 100644
>>> --- 
>>> a/EmbeddedPkg/Library/PrePiMemoryAllocationLib/MemoryAllocationLib.c
>>> +++ b/EmbeddedPkg/Library/PrePiMemoryAllocationLib/MemoryAllocationLi
>>> +++ b.c
>>> @@ -308,7 +308,7 @@ ReallocatePool (
>>>    if (OldBuffer != NULL) {
>>>      HandOffHob = GetHobList ();
>>>      ASSERT (((EFI_PHYSICAL_ADDRESS)(UINTN)OldBuffer >= HandOffHob->EfiMemoryBottom));
>>> -    ASSERT (((EFI_PHYSICAL_ADDRESS)(UINTN)(OldBuffer + OldSize) <= HandOffHob->EfiFreeMemoryBottom));
>>> +    ASSERT (((EFI_PHYSICAL_ADDRESS)(UINTN)((UINTN)OldBuffer + 
>>> + OldSize) <= HandOffHob->EfiFreeMemoryBottom));
>>
>>
>> Do we really need two UINTN casts here?
>>
> 
> No, that should not be necessary. In
> <https://edk2.groups.io/g/devel/message/115050> (msgid <01296486-5b14-4b73-b9dc-777a723548cf@redhat.com>), I recommended:
> 
> --------------
> We should replace (UINTN)(OldBuffer + OldSize) with ((UINTN)OldBuffer + OldSize).
> --------------
> 
> Note that the text to remove includes the original (UINTN) cast.
> 
> Laszlo
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115235): https://edk2.groups.io/g/devel/message/115235
Mute This Topic: https://groups.io/mt/104193285/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-