Re: [edk2-devel] [PATCH v1 18/19] ArmPlatformPkg: Fix comparison of constants warning

2019-11-21 Thread Leif Lindholm
On Thu, Nov 21, 2019 at 14:09:41 +0100, Ard Biesheuvel wrote:
> > > I don't like this at all.
> > >
> > > If two expressions happen to evaluate to the same constant, we should
> > > be able to compare them using C code.
> > >
> > > I think we should disable the warning instead.
> >
> > I agree with that as a general rule, and agree that disabling the
> > warning may be preferable in other contexts.
> >
> > But I somewhat disagree with referring to *this instance* as two
> > expressions evaluating to the same constant. This is entirely a
> > preprocessor event, somewhat obscured by the Pcd syntax.
> > The test is "has the default baudrate been requested or has a specific
> > one been requested?".
> 
> That may be true, but moving from C conditionals to CPP conditionals
> obscures the code, and reduces test coverage, so I think it should be
> avoided when possible.

I don't see what test coverage this would change. Even in the NOOPT
case, only the codegen would be affected - only one of the two cases
would be reachable. In all other cases, the compiler would go
  if (115200 != 0) {
  /* Oh, I'll just ignore any else clauses then ... */

I think the pre-existing code obscures what is actually going on and
that the change clarifies it (but could be prettified).

Now, of course, why we have a platform-specific (and platform-global)
override variable to make a library ignore its inputs is another
interesting question.

So. Rock-paper-scissors or coin-toss?

/
Leif

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#51068): https://edk2.groups.io/g/devel/message/51068
Mute This Topic: https://groups.io/mt/32999801/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH v1 18/19] ArmPlatformPkg: Fix comparison of constants warning

2019-11-21 Thread Ard Biesheuvel
On Thu, 21 Nov 2019 at 16:15, Leif Lindholm  wrote:
>
> On Thu, Nov 21, 2019 at 14:09:41 +0100, Ard Biesheuvel wrote:
> > > > I don't like this at all.
> > > >
> > > > If two expressions happen to evaluate to the same constant, we should
> > > > be able to compare them using C code.
> > > >
> > > > I think we should disable the warning instead.
> > >
> > > I agree with that as a general rule, and agree that disabling the
> > > warning may be preferable in other contexts.
> > >
> > > But I somewhat disagree with referring to *this instance* as two
> > > expressions evaluating to the same constant. This is entirely a
> > > preprocessor event, somewhat obscured by the Pcd syntax.
> > > The test is "has the default baudrate been requested or has a specific
> > > one been requested?".
> >
> > That may be true, but moving from C conditionals to CPP conditionals
> > obscures the code, and reduces test coverage, so I think it should be
> > avoided when possible.
>
> I don't see what test coverage this would change. Even in the NOOPT
> case, only the codegen would be affected - only one of the two cases
> would be reachable. In all other cases, the compiler would go
>   if (115200 != 0) {
>   /* Oh, I'll just ignore any else clauses then ... */
>
> I think the pre-existing code obscures what is actually going on and
> that the change clarifies it (but could be prettified).
>
> Now, of course, why we have a platform-specific (and platform-global)
> override variable to make a library ignore its inputs is another
> interesting question.
>
> So. Rock-paper-scissors or coin-toss?
>

Yes!

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#51070): https://edk2.groups.io/g/devel/message/51070
Mute This Topic: https://groups.io/mt/32999801/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH v1 18/19] ArmPlatformPkg: Fix comparison of constants warning

2019-11-21 Thread Ard Biesheuvel
On Thu, 21 Nov 2019 at 13:53, Leif Lindholm  wrote:
>
> On Thu, Nov 21, 2019 at 13:39:55 +0100, Ard Biesheuvel wrote:
> > On Thu, 21 Nov 2019 at 13:35, Leif Lindholm  
> > wrote:
> > >
> > > On Fri, Aug 23, 2019 at 11:55:38 +0100, Sami Mujawar wrote:
> > > > The VS2017 compiler reports 'warning C6326: potential
> > > > comparison of a constant with another constant' when
> > > > a fixed PCD value is compared with a constant value.
> > > >
> > > > The faulting code is as marked by '-->' below:
> > > >
> > > > --> if (FixedPcdGet32 (PL011UartInteger) != 0) {
> > > >   Integer = FixedPcdGet32 (PL011UartInteger);
> > > >   Fractional = FixedPcdGet32 (PL011UartFractional);
> > > > } else {
> > > > ...
> > > >
> > > > The macro FixedPcdGet32 (PL011UartInteger) evaluates
> > > > to a macro _PCD_VALUE_PL011UartInteger that is defined
> > > > by the build system to represent the UART Integer
> > > > value. Therefore, the VS2017 compiler reports the above
> > > > warning.
> > > >
> > > > Fix this warning by enclosing the code in appropriate
> > > >  #if .. #else .. #endif directives.
> > > >
> > > > Signed-off-by: Sami Mujawar 
> > >
> > > Reviewed-by: Leif Lindholm 
> > >
> >
> > I don't like this at all.
> >
> > If two expressions happen to evaluate to the same constant, we should
> > be able to compare them using C code.
> >
> > I think we should disable the warning instead.
>
> I agree with that as a general rule, and agree that disabling the
> warning may be preferable in other contexts.
>
> But I somewhat disagree with referring to *this instance* as two
> expressions evaluating to the same constant. This is entirely a
> preprocessor event, somewhat obscured by the Pcd syntax.
> The test is "has the default baudrate been requested or has a specific
> one been requested?".
>

That may be true, but moving from C conditionals to CPP conditionals
obscures the code, and reduces test coverage, so I think it should be
avoided when possible.

> Now, that might be more cleanly described as a macro than inline like
> this - but I think the compiler has usefully warned us about weirdly
> written code. So I would really not want to disable the warning on a
> global level, and would prefer not to do so here.
>
> /
> Leif
>
> >
> > > > ---
> > > >  ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c | 6 +++---
> > > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c 
> > > > b/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
> > > > index 
> > > > 2d3c279cce49304959953ec4a34b50e09a7d0045..dabf099b9bc82e1fb1bd5a2eae3fa4b5878a9e07
> > > >  100644
> > > > --- a/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
> > > > +++ b/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
> > > > @@ -174,10 +174,10 @@ PL011UartInitializePort (
> > > >//
> > > >
> > > >// If PL011 Integer value has been defined then always ignore the 
> > > > BAUD rate
> > > > -  if (FixedPcdGet32 (PL011UartInteger) != 0) {
> > > > +#if (FixedPcdGet32 (PL011UartInteger) != 0)
> > > >  Integer = FixedPcdGet32 (PL011UartInteger);
> > > >  Fractional = FixedPcdGet32 (PL011UartFractional);
> > > > -  } else {
> > > > +#else
> > > >  // If BAUD rate is zero then replace it with the system default 
> > > > value
> > > >  if (*BaudRate == 0) {
> > > >*BaudRate = FixedPcdGet32 (PcdSerialBaudRate);
> > > > @@ -197,7 +197,7 @@ PL011UartInitializePort (
> > > >  Divisor = (UINT32)DivisorValue;
> > > >  Integer = Divisor >> FRACTION_PART_SIZE_IN_BITS;
> > > >  Fractional = Divisor & FRACTION_PART_MASK;
> > > > -  }
> > > > +#endif
> > > >
> > > >//
> > > >// If PL011 is already initialized, check the current settings
> > > > --
> > > > 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
> > > >
> > > >

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#51059): https://edk2.groups.io/g/devel/message/51059
Mute This Topic: https://groups.io/mt/32999801/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH v1 18/19] ArmPlatformPkg: Fix comparison of constants warning

2019-11-21 Thread Leif Lindholm
On Thu, Nov 21, 2019 at 13:39:55 +0100, Ard Biesheuvel wrote:
> On Thu, 21 Nov 2019 at 13:35, Leif Lindholm  wrote:
> >
> > On Fri, Aug 23, 2019 at 11:55:38 +0100, Sami Mujawar wrote:
> > > The VS2017 compiler reports 'warning C6326: potential
> > > comparison of a constant with another constant' when
> > > a fixed PCD value is compared with a constant value.
> > >
> > > The faulting code is as marked by '-->' below:
> > >
> > > --> if (FixedPcdGet32 (PL011UartInteger) != 0) {
> > >   Integer = FixedPcdGet32 (PL011UartInteger);
> > >   Fractional = FixedPcdGet32 (PL011UartFractional);
> > > } else {
> > > ...
> > >
> > > The macro FixedPcdGet32 (PL011UartInteger) evaluates
> > > to a macro _PCD_VALUE_PL011UartInteger that is defined
> > > by the build system to represent the UART Integer
> > > value. Therefore, the VS2017 compiler reports the above
> > > warning.
> > >
> > > Fix this warning by enclosing the code in appropriate
> > >  #if .. #else .. #endif directives.
> > >
> > > Signed-off-by: Sami Mujawar 
> >
> > Reviewed-by: Leif Lindholm 
> >
> 
> I don't like this at all.
> 
> If two expressions happen to evaluate to the same constant, we should
> be able to compare them using C code.
>
> I think we should disable the warning instead.

I agree with that as a general rule, and agree that disabling the
warning may be preferable in other contexts.

But I somewhat disagree with referring to *this instance* as two
expressions evaluating to the same constant. This is entirely a
preprocessor event, somewhat obscured by the Pcd syntax.
The test is "has the default baudrate been requested or has a specific
one been requested?".

Now, that might be more cleanly described as a macro than inline like
this - but I think the compiler has usefully warned us about weirdly
written code. So I would really not want to disable the warning on a
global level, and would prefer not to do so here.

/
Leif
 
> 
> > > ---
> > >  ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c | 6 +++---
> > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c 
> > > b/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
> > > index 
> > > 2d3c279cce49304959953ec4a34b50e09a7d0045..dabf099b9bc82e1fb1bd5a2eae3fa4b5878a9e07
> > >  100644
> > > --- a/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
> > > +++ b/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
> > > @@ -174,10 +174,10 @@ PL011UartInitializePort (
> > >//
> > >
> > >// If PL011 Integer value has been defined then always ignore the BAUD 
> > > rate
> > > -  if (FixedPcdGet32 (PL011UartInteger) != 0) {
> > > +#if (FixedPcdGet32 (PL011UartInteger) != 0)
> > >  Integer = FixedPcdGet32 (PL011UartInteger);
> > >  Fractional = FixedPcdGet32 (PL011UartFractional);
> > > -  } else {
> > > +#else
> > >  // If BAUD rate is zero then replace it with the system default value
> > >  if (*BaudRate == 0) {
> > >*BaudRate = FixedPcdGet32 (PcdSerialBaudRate);
> > > @@ -197,7 +197,7 @@ PL011UartInitializePort (
> > >  Divisor = (UINT32)DivisorValue;
> > >  Integer = Divisor >> FRACTION_PART_SIZE_IN_BITS;
> > >  Fractional = Divisor & FRACTION_PART_MASK;
> > > -  }
> > > +#endif
> > >
> > >//
> > >// If PL011 is already initialized, check the current settings
> > > --
> > > 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
> > >
> > >

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#51058): https://edk2.groups.io/g/devel/message/51058
Mute This Topic: https://groups.io/mt/32999801/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH v1 18/19] ArmPlatformPkg: Fix comparison of constants warning

2019-11-21 Thread Ard Biesheuvel
On Thu, 21 Nov 2019 at 13:35, Leif Lindholm  wrote:
>
> On Fri, Aug 23, 2019 at 11:55:38 +0100, Sami Mujawar wrote:
> > The VS2017 compiler reports 'warning C6326: potential
> > comparison of a constant with another constant' when
> > a fixed PCD value is compared with a constant value.
> >
> > The faulting code is as marked by '-->' below:
> >
> > --> if (FixedPcdGet32 (PL011UartInteger) != 0) {
> >   Integer = FixedPcdGet32 (PL011UartInteger);
> >   Fractional = FixedPcdGet32 (PL011UartFractional);
> > } else {
> > ...
> >
> > The macro FixedPcdGet32 (PL011UartInteger) evaluates
> > to a macro _PCD_VALUE_PL011UartInteger that is defined
> > by the build system to represent the UART Integer
> > value. Therefore, the VS2017 compiler reports the above
> > warning.
> >
> > Fix this warning by enclosing the code in appropriate
> >  #if .. #else .. #endif directives.
> >
> > Signed-off-by: Sami Mujawar 
>
> Reviewed-by: Leif Lindholm 
>

I don't like this at all.

If two expressions happen to evaluate to the same constant, we should
be able to compare them using C code.

I think we should disable the warning instead.


> > ---
> >  ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c 
> > b/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
> > index 
> > 2d3c279cce49304959953ec4a34b50e09a7d0045..dabf099b9bc82e1fb1bd5a2eae3fa4b5878a9e07
> >  100644
> > --- a/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
> > +++ b/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
> > @@ -174,10 +174,10 @@ PL011UartInitializePort (
> >//
> >
> >// If PL011 Integer value has been defined then always ignore the BAUD 
> > rate
> > -  if (FixedPcdGet32 (PL011UartInteger) != 0) {
> > +#if (FixedPcdGet32 (PL011UartInteger) != 0)
> >  Integer = FixedPcdGet32 (PL011UartInteger);
> >  Fractional = FixedPcdGet32 (PL011UartFractional);
> > -  } else {
> > +#else
> >  // If BAUD rate is zero then replace it with the system default value
> >  if (*BaudRate == 0) {
> >*BaudRate = FixedPcdGet32 (PcdSerialBaudRate);
> > @@ -197,7 +197,7 @@ PL011UartInitializePort (
> >  Divisor = (UINT32)DivisorValue;
> >  Integer = Divisor >> FRACTION_PART_SIZE_IN_BITS;
> >  Fractional = Divisor & FRACTION_PART_MASK;
> > -  }
> > +#endif
> >
> >//
> >// If PL011 is already initialized, check the current settings
> > --
> > 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
> >
> >

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#51057): https://edk2.groups.io/g/devel/message/51057
Mute This Topic: https://groups.io/mt/32999801/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH v1 18/19] ArmPlatformPkg: Fix comparison of constants warning

2019-11-21 Thread Leif Lindholm
On Fri, Aug 23, 2019 at 11:55:38 +0100, Sami Mujawar wrote:
> The VS2017 compiler reports 'warning C6326: potential
> comparison of a constant with another constant' when
> a fixed PCD value is compared with a constant value.
> 
> The faulting code is as marked by '-->' below:
> 
> --> if (FixedPcdGet32 (PL011UartInteger) != 0) {
>   Integer = FixedPcdGet32 (PL011UartInteger);
>   Fractional = FixedPcdGet32 (PL011UartFractional);
> } else {
> ...
> 
> The macro FixedPcdGet32 (PL011UartInteger) evaluates
> to a macro _PCD_VALUE_PL011UartInteger that is defined
> by the build system to represent the UART Integer
> value. Therefore, the VS2017 compiler reports the above
> warning.
> 
> Fix this warning by enclosing the code in appropriate
>  #if .. #else .. #endif directives.
> 
> Signed-off-by: Sami Mujawar 

Reviewed-by: Leif Lindholm 

> ---
>  ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c 
> b/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
> index 
> 2d3c279cce49304959953ec4a34b50e09a7d0045..dabf099b9bc82e1fb1bd5a2eae3fa4b5878a9e07
>  100644
> --- a/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
> +++ b/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
> @@ -174,10 +174,10 @@ PL011UartInitializePort (
>//
>  
>// If PL011 Integer value has been defined then always ignore the BAUD rate
> -  if (FixedPcdGet32 (PL011UartInteger) != 0) {
> +#if (FixedPcdGet32 (PL011UartInteger) != 0)
>  Integer = FixedPcdGet32 (PL011UartInteger);
>  Fractional = FixedPcdGet32 (PL011UartFractional);
> -  } else {
> +#else
>  // If BAUD rate is zero then replace it with the system default value
>  if (*BaudRate == 0) {
>*BaudRate = FixedPcdGet32 (PcdSerialBaudRate);
> @@ -197,7 +197,7 @@ PL011UartInitializePort (
>  Divisor = (UINT32)DivisorValue;
>  Integer = Divisor >> FRACTION_PART_SIZE_IN_BITS;
>  Fractional = Divisor & FRACTION_PART_MASK;
> -  }
> +#endif
>  
>//
>// If PL011 is already initialized, check the current settings
> -- 
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
> 
> 

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#51056): https://edk2.groups.io/g/devel/message/51056
Mute This Topic: https://groups.io/mt/32999801/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH v1 18/19] ArmPlatformPkg: Fix comparison of constants warning

2019-08-23 Thread Alexei Fedorov
Reviewed-by: Alexei Fedorov 


Alexei


From: Sami Mujawar 
Sent: 23 August 2019 11:55
To: devel@edk2.groups.io 
Cc: Sami Mujawar ; Alexei Fedorov 
; ard.biesheu...@linaro.org 
; leif.lindh...@linaro.org 
; Matteo Carlini ; nd 

Subject: [PATCH v1 18/19] ArmPlatformPkg: Fix comparison of constants warning

The VS2017 compiler reports 'warning C6326: potential
comparison of a constant with another constant' when
a fixed PCD value is compared with a constant value.

The faulting code is as marked by '-->' below:

--> if (FixedPcdGet32 (PL011UartInteger) != 0) {
  Integer = FixedPcdGet32 (PL011UartInteger);
  Fractional = FixedPcdGet32 (PL011UartFractional);
} else {
...

The macro FixedPcdGet32 (PL011UartInteger) evaluates
to a macro _PCD_VALUE_PL011UartInteger that is defined
by the build system to represent the UART Integer
value. Therefore, the VS2017 compiler reports the above
warning.

Fix this warning by enclosing the code in appropriate
 #if .. #else .. #endif directives.

Signed-off-by: Sami Mujawar 
---
 ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c 
b/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
index 
2d3c279cce49304959953ec4a34b50e09a7d0045..dabf099b9bc82e1fb1bd5a2eae3fa4b5878a9e07
 100644
--- a/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
+++ b/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
@@ -174,10 +174,10 @@ PL011UartInitializePort (
   //

   // If PL011 Integer value has been defined then always ignore the BAUD rate
-  if (FixedPcdGet32 (PL011UartInteger) != 0) {
+#if (FixedPcdGet32 (PL011UartInteger) != 0)
 Integer = FixedPcdGet32 (PL011UartInteger);
 Fractional = FixedPcdGet32 (PL011UartFractional);
-  } else {
+#else
 // If BAUD rate is zero then replace it with the system default value
 if (*BaudRate == 0) {
   *BaudRate = FixedPcdGet32 (PcdSerialBaudRate);
@@ -197,7 +197,7 @@ PL011UartInitializePort (
 Divisor = (UINT32)DivisorValue;
 Integer = Divisor >> FRACTION_PART_SIZE_IN_BITS;
 Fractional = Divisor & FRACTION_PART_MASK;
-  }
+#endif

   //
   // If PL011 is already initialized, check the current settings
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#46285): https://edk2.groups.io/g/devel/message/46285
Mute This Topic: https://groups.io/mt/32999801/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



[edk2-devel] [PATCH v1 18/19] ArmPlatformPkg: Fix comparison of constants warning

2019-08-23 Thread Sami Mujawar
The VS2017 compiler reports 'warning C6326: potential
comparison of a constant with another constant' when
a fixed PCD value is compared with a constant value.

The faulting code is as marked by '-->' below:

--> if (FixedPcdGet32 (PL011UartInteger) != 0) {
  Integer = FixedPcdGet32 (PL011UartInteger);
  Fractional = FixedPcdGet32 (PL011UartFractional);
} else {
...

The macro FixedPcdGet32 (PL011UartInteger) evaluates
to a macro _PCD_VALUE_PL011UartInteger that is defined
by the build system to represent the UART Integer
value. Therefore, the VS2017 compiler reports the above
warning.

Fix this warning by enclosing the code in appropriate
 #if .. #else .. #endif directives.

Signed-off-by: Sami Mujawar 
---
 ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c 
b/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
index 
2d3c279cce49304959953ec4a34b50e09a7d0045..dabf099b9bc82e1fb1bd5a2eae3fa4b5878a9e07
 100644
--- a/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
+++ b/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
@@ -174,10 +174,10 @@ PL011UartInitializePort (
   //
 
   // If PL011 Integer value has been defined then always ignore the BAUD rate
-  if (FixedPcdGet32 (PL011UartInteger) != 0) {
+#if (FixedPcdGet32 (PL011UartInteger) != 0)
 Integer = FixedPcdGet32 (PL011UartInteger);
 Fractional = FixedPcdGet32 (PL011UartFractional);
-  } else {
+#else
 // If BAUD rate is zero then replace it with the system default value
 if (*BaudRate == 0) {
   *BaudRate = FixedPcdGet32 (PcdSerialBaudRate);
@@ -197,7 +197,7 @@ PL011UartInitializePort (
 Divisor = (UINT32)DivisorValue;
 Integer = Divisor >> FRACTION_PART_SIZE_IN_BITS;
 Fractional = Divisor & FRACTION_PART_MASK;
-  }
+#endif
 
   //
   // If PL011 is already initialized, check the current settings
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#46278): https://edk2.groups.io/g/devel/message/46278
Mute This Topic: https://groups.io/mt/32999801/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-