Re: [PATCH 1/2] MdeModulePkg/PciBusDxe: Prevent truncating constant values.

[Andrew Fish <afish@apple.com>]

> On Feb 27, 2018, at 10:42 AM, Laszlo Ersek <lersek@redhat.com> wrote:
> 
> On 02/27/18 17:49, Marvin Häuser wrote:
>> The toolcahin VS2015x86 issues warnings when truncating constant
>> values. Explicitely cast such to avoid it.
>> 
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Marvin Haeuser <Marvin.Haeuser@outlook.com>
>> ---
>> MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
>> index 2f713fcee95e..a752853f3e9e 100644
>> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
>> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
>> @@ -1936,7 +1936,7 @@ ProgramP2C (
>>                    &BridgeControl
>>                    );
>> 
>> -      BridgeControl &= (UINT16) ~PCI_CARD_PREFETCHABLE_MEMORY_0_ENABLE;
>> +      BridgeControl &= (UINT16) ~(UINT16)PCI_CARD_PREFETCHABLE_MEMORY_0_ENABLE;
>>       PciIo->Pci.Write (
>>                    PciIo,
>>                    EfiPciIoWidthUint16,
>> @@ -2005,7 +2005,7 @@ ProgramP2C (
>>                    &BridgeControl
>>                    );
>> 
>> -      BridgeControl &= (UINT16) ~(PCI_CARD_PREFETCHABLE_MEMORY_1_ENABLE);
>> +      BridgeControl &= (UINT16) ~(UINT16)(PCI_CARD_PREFETCHABLE_MEMORY_1_ENABLE);
>>       PciIo->Pci.Write (
>>                    PciIo,
>>                    EfiPciIoWidthUint16,
>> 
> 
> My recommendation is the same as for:
> 
>  [edk2] [PATCH 2/2] MdeModulePkg/BaseSerialPortLib16550: Prevent
>                     truncating constant values.
> 
> #define PCI_CARD_PREFETCHABLE_MEMORY_0_ENABLE BIT8
> #define PCI_CARD_PREFETCHABLE_MEMORY_1_ENABLE BIT9
> 
> #define  BIT8     0x00000100
> #define  BIT9     0x00000200
> 

Laszlo,

Stupid question? Would making BIT8 0x00000100U help? I notice we use ULL for the larger ones, and I don't remember why we don't use U for the ones that fit into a int?

Thanks,

Andrew Fish

> So I suggest (UINT32) for the casts.
> 
> 
> I'd also suggest a better subject line / commit message. Can we say:
> 
>  Package/Module: avoid bit-negating signed integer constants
> 
> and then the commit message could quote the VS2015x86 warnings.
> 
> Again, just my two cents; I defer to the MdeModulePkg maintainers.
> 
> Thanks!
> Laszlo
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org <mailto:edk2-devel@lists.01.org>
> https://lists.01.org/mailman/listinfo/edk2-devel <https://lists.01.org/mailman/listinfo/edk2-devel>