diff mbox

[edk2] ArmVirtPkg/FdtPciPcdProducerLib: zero init local var to please GCC 4.8

Message ID 1473062175-1567-1-git-send-email-ard.biesheuvel@linaro.org
State Accepted
Commit 70c368e26f79ba4d0867090b88d48b86884e6ab2
Headers show

Commit Message

Ard Biesheuvel Sept. 5, 2016, 7:56 a.m. UTC
GCC 4.8 in RELEASE mode complains about GetPciIoTranslation () potentially
not assigning IoTranslation, but does not notice that it returns failure in
this case, which means IoTranslation is never referenced *unless* it has
been assigned. So simply set IoTranslation to zero to help the compiler.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

---
 ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.c | 1 +
 1 file changed, 1 insertion(+)

-- 
2.7.4

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Laszlo Ersek Sept. 5, 2016, 11:41 a.m. UTC | #1
On 09/05/16 09:56, Ard Biesheuvel wrote:
> GCC 4.8 in RELEASE mode complains about GetPciIoTranslation () potentially

> not assigning IoTranslation, but does not notice that it returns failure in

> this case, which means IoTranslation is never referenced *unless* it has

> been assigned. So simply set IoTranslation to zero to help the compiler.

> 

> Contributed-under: TianoCore Contribution Agreement 1.0

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---

>  ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.c | 1 +

>  1 file changed, 1 insertion(+)

> 

> diff --git a/ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.c b/ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.c

> index 10b47560cb9c..ea27cda7b77c 100644

> --- a/ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.c

> +++ b/ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.c

> @@ -128,6 +128,7 @@ FdtPciPcdProducerLibConstructor (

>  

>        PcdSetBool (PcdPciDisableBusEnumeration, FALSE);

>  

> +      IoTranslation = 0;

>        RetStatus = GetPciIoTranslation (FdtClient, Node, &IoTranslation);

>        if (!RETURN_ERROR (RetStatus)) {

>            PcdSet64 (PcdPciIoTranslation, IoTranslation);

> 


Ouch. This is exactly why "IoTranslation = 0" appeared at the beginning,
in commit 65bb13b0fd7f ("ArmVirtualizationPkg/VirtFdtDxe: parse
"pci-host-ecam-generic" properties"), and why we've been carrying it
around, most recently in commit d4cb9a30494d ("ArmVirtPkg: implement
FdtPciHostBridgeLib"). We forgot about it in commit c8f1a75aa997
("ArmVirtPkg/FdtPciPcdProducerLib: add handling of PcdPciIoTranslation").

Reviewed-by: Laszlo Ersek <lersek@redhat.com>


Thanks
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel Sept. 5, 2016, 11:44 a.m. UTC | #2
On 5 September 2016 at 12:41, Laszlo Ersek <lersek@redhat.com> wrote:
> On 09/05/16 09:56, Ard Biesheuvel wrote:

>> GCC 4.8 in RELEASE mode complains about GetPciIoTranslation () potentially

>> not assigning IoTranslation, but does not notice that it returns failure in

>> this case, which means IoTranslation is never referenced *unless* it has

>> been assigned. So simply set IoTranslation to zero to help the compiler.

>>

>> Contributed-under: TianoCore Contribution Agreement 1.0

>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

>> ---

>>  ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.c | 1 +

>>  1 file changed, 1 insertion(+)

>>

>> diff --git a/ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.c b/ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.c

>> index 10b47560cb9c..ea27cda7b77c 100644

>> --- a/ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.c

>> +++ b/ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.c

>> @@ -128,6 +128,7 @@ FdtPciPcdProducerLibConstructor (

>>

>>        PcdSetBool (PcdPciDisableBusEnumeration, FALSE);

>>

>> +      IoTranslation = 0;

>>        RetStatus = GetPciIoTranslation (FdtClient, Node, &IoTranslation);

>>        if (!RETURN_ERROR (RetStatus)) {

>>            PcdSet64 (PcdPciIoTranslation, IoTranslation);

>>

>

> Ouch. This is exactly why "IoTranslation = 0" appeared at the beginning,

> in commit 65bb13b0fd7f ("ArmVirtualizationPkg/VirtFdtDxe: parse

> "pci-host-ecam-generic" properties"), and why we've been carrying it

> around, most recently in commit d4cb9a30494d ("ArmVirtPkg: implement

> FdtPciHostBridgeLib"). We forgot about it in commit c8f1a75aa997

> ("ArmVirtPkg/FdtPciPcdProducerLib: add handling of PcdPciIoTranslation").

>

> Reviewed-by: Laszlo Ersek <lersek@redhat.com>

>


Pushed, thanks.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
diff mbox

Patch

diff --git a/ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.c b/ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.c
index 10b47560cb9c..ea27cda7b77c 100644
--- a/ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.c
+++ b/ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.c
@@ -128,6 +128,7 @@  FdtPciPcdProducerLibConstructor (
 
       PcdSetBool (PcdPciDisableBusEnumeration, FALSE);
 
+      IoTranslation = 0;
       RetStatus = GetPciIoTranslation (FdtClient, Node, &IoTranslation);
       if (!RETURN_ERROR (RetStatus)) {
           PcdSet64 (PcdPciIoTranslation, IoTranslation);