Re: [PATCH] hw/net/i82596: Correct command bitmask (CID 1419392)

[Jason Wang <jasowang@redhat.com>]

On 2020/3/13 下午7:01, Peter Maydell wrote:
> On Fri, 14 Feb 2020 at 00:48, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>> The command is 32-bit, but we are loading the 16 upper bits with
>> the 'get_uint16(s->scb + 2)' call.
>>
>> Once shifted by 16, the command bits match the status bits:
>>
>> - Command
>>    Bit 31 ACK-CX   Acknowledges that the CU completed an Action Command.
>>    Bit 30 ACK-FR   Acknowledges that the RU received a frame.
>>    Bit 29 ACK-CNA  Acknowledges that the Command Unit became not active.
>>    Bit 28 ACK-RNR  Acknowledges that the Receive Unit became not ready.
>>
>> - Status
>>    Bit 15 CX       The CU finished executing a command with its I(interrupt) bit set.
>>    Bit 14 FR       The RU finished receiving a frame.
>>    Bit 13 CNA      The Command Unit left the Active state.
>>    Bit 12 RNR      The Receive Unit left the Ready state.
>>
>> Add the SCB_COMMAND_ACK_MASK definition to simplify the code.
>>
>> This fixes Coverity 1419392 (CONSTANT_EXPRESSION_RESULT):
>>
>>    /hw/net/i82596.c: 352 in examine_scb()
>>    346         cuc = (command >> 8) & 0x7;
>>    347         ruc = (command >> 4) & 0x7;
>>    348         DBG(printf("MAIN COMMAND %04x  cuc %02x ruc %02x\n", command, cuc, ruc));
>>    349         /* and clear the scb command word */
>>    350         set_uint16(s->scb + 2, 0);
>>    351
>>    >>>     CID 1419392:    (CONSTANT_EXPRESSION_RESULT)
>>    >>>     "command & (2147483648UL /* 1UL << 31 */)" is always 0 regardless of the values of its operands. This occurs as the logical operand of "if".
>>    352         if (command & BIT(31))      /* ACK-CX */
>>    353             s->scb_status &= ~SCB_STATUS_CX;
>>    >>>     CID 1419392:    (CONSTANT_EXPRESSION_RESULT)
>>    >>>     "command & (1073741824UL /* 1UL << 30 */)" is always 0 regardless of the values of its operands. This occurs as the logical operand of "if".
>>    354         if (command & BIT(30))      /*ACK-FR */
>>    355             s->scb_status &= ~SCB_STATUS_FR;
>>    >>>     CID 1419392:    (CONSTANT_EXPRESSION_RESULT)
>>    >>>     "command & (536870912UL /* 1UL << 29 */)" is always 0 regardless of the values of its operands. This occurs as the logical operand of "if".
>>    356         if (command & BIT(29))      /*ACK-CNA */
>>    357             s->scb_status &= ~SCB_STATUS_CNA;
>>    >>>     CID 1419392:    (CONSTANT_EXPRESSION_RESULT)
>>    >>>     "command & (268435456UL /* 1UL << 28 */)" is always 0 regardless of the values of its operands. This occurs as the logical operand of "if".
>>    358         if (command & BIT(28))      /*ACK-RNR */
>>    359             s->scb_status &= ~SCB_STATUS_RNR;
>>
>> Fixes: Covertiy CID 1419392 (commit 376b851909)
> ("Coverity")
>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>
> Jason, are you planning to pick this one up?


Yes. queued.

Thanks


> thanks
> -- PMM
>