Re: [edk2] [PATCH 3/8] PcAtChipsetPkg: remove unitialized variable warnings

[Zeng, Star]

On 2015/12/17 18:00, Ard Biesheuvel wrote:

LcrParity and LcrStop may end up being referenced without being
initialized, so make sure they always have a value.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
  PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c | 8 ++--
  1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c 
b/PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c
index 8656785347b2..5698e935b01f 100644
--- a/PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c
+++ b/PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c
@@ -432,6 +432,7 @@ SerialPortSetAttributes (

switch (*Parity) {
  case NoParity:
+case DefaultParity:
LcrParity = 0;
break;

@@ -450,13 +451,11 @@ SerialPortSetAttributes (
  case MarkParity:
LcrParity = 5;
break;
-
-default:
-  break;
}

switch (*StopBits) {
  case OneStopBit:
+case DefaultStopBits:
LcrStop = 0;
break;

@@ -464,9 +463,6 @@ SerialPortSetAttributes (
  case TwoStopBits:
LcrStop = 1;
break;
-
-default:
-  break;
}

//



At the beginning of SerialPortSetAttributes(), there has been already 
code below. The uninitialized case should never happen, is the compiler 
not intelligent enough to catch logic?


  if (*Parity == DefaultParity) {
*Parity = NoParity;
  }

  if (*StopBits == DefaultStopBits) {
*StopBits = OneStopBit;
  }

Thanks,
Star


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

Re: [edk2] [PATCH 3/8] PcAtChipsetPkg: remove unitialized variable warnings

[Ard Biesheuvel]

On 17 December 2015 at 14:59, Zeng, Star  wrote:
> On 2015/12/17 18:00, Ard Biesheuvel wrote:
>>
>> LcrParity and LcrStop may end up being referenced without being
>> initialized, so make sure they always have a value.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel 
>> ---
>>   PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c | 8 ++--
>>   1 file changed, 2 insertions(+), 6 deletions(-)
>>
>> diff --git a/PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c
>> b/PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c
>> index 8656785347b2..5698e935b01f 100644
>> --- a/PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c
>> +++ b/PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c
>> @@ -432,6 +432,7 @@ SerialPortSetAttributes (
>>
>> switch (*Parity) {
>>   case NoParity:
>> +case DefaultParity:
>> LcrParity = 0;
>> break;
>>
>> @@ -450,13 +451,11 @@ SerialPortSetAttributes (
>>   case MarkParity:
>> LcrParity = 5;
>> break;
>> -
>> -default:
>> -  break;
>> }
>>
>> switch (*StopBits) {
>>   case OneStopBit:
>> +case DefaultStopBits:
>> LcrStop = 0;
>> break;
>>
>> @@ -464,9 +463,6 @@ SerialPortSetAttributes (
>>   case TwoStopBits:
>> LcrStop = 1;
>> break;
>> -
>> -default:
>> -  break;
>> }
>>
>> //
>>
>
> At the beginning of SerialPortSetAttributes(), there has been already code
> below. The uninitialized case should never happen, is the compiler not
> intelligent enough to catch logic?
>
>   if (*Parity == DefaultParity) {
> *Parity = NoParity;
>   }
>
>   if (*StopBits == DefaultStopBits) {
> *StopBits = OneStopBit;
>   }
>

That is a very good point. I already wondered why this compiler was
the first one to spot it.

The actual error is as follows:
PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c:454:5: error:
variable 'LcrParity' is used uninitialized whenever switch default is
taken

Anyway, having a default case that does not set LcrParity or LcrStop
looks a bit strange anyway, so perhaps the fix can be taken anyway?
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel