On 17 December 2015 at 14:59, Zeng, Star <star.z...@intel.com> 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 <ard.biesheu...@linaro.org>
>> ---
>>   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

Reply via email to