On 20/11/2014 08:38, Gonglei wrote:
> On 2014/11/20 15:08, Paolo Bonzini wrote:
>
>>
>>
>> On 20/11/2014 07:44, Gonglei wrote:
>>> Maybe not, since two branch are "if and else if" not "if and else",
>>> so this change make the below code segment's wide ...
>>>>> bcnt = 4096 - GET_FIELD(tmd.length, TMDL, BCNT);
>>>>> s->phys_mem_read(s->dma_opaque, PHYSADDR(s, tmd.tbadr),
>>>>> s->buffer + s->xmit_pos, bcnt, CSR_BSWP(s));
>>>>> s->xmit_pos += bcnt;
>>> ... more extensive.
>>
>> After your patch that fixes the coverity report, they are
>>
>> if (a && b)
>> else if (b)
>>
>> so you can change it to
>>
>> if (!b) goto txdone;
>> if (a) ...
>> else ...
>>
>> and then
>>
>> if (!b) goto txdone;
>> <common part>
>> if (!a) {
>> <extra part from else>
>> }
>>
>> Paolo
>
> I know your mean now, thanks ;)
> What about this below way? Maybe more clear.
As you prefer.
Paolo
> if (s->xmit_pos < 0) {
> goto txdone;
> }
> int bcnt = 4096 - GET_FIELD(tmd.length, TMDL, BCNT);
> s->phys_mem_read(s->dma_opaque, PHYSADDR(s, tmd.tbadr),
> s->buffer + s->xmit_pos, bcnt, CSR_BSWP(s));
> s->xmit_pos += bcnt;
>
> if (!GET_FIELD(tmd.status, TMDS, ENP)) {
> goto txdone;
> }
>
> #ifdef PCNET_DEBUG
> printf("pcnet_transmit size=%d\n", s->xmit_pos);
> #endif
> if (CSR_LOOP(s)) {
> if (BCR_SWSTYLE(s) == 1)
> add_crc = !GET_FIELD(tmd.status, TMDS, NOFCS);
> s->looptest = add_crc ? PCNET_LOOPTEST_CRC : PCNET_LOOPTEST_NOCRC;
> pcnet_receive(qemu_get_queue(s->nic), s->buffer, s->xmit_pos);
> s->looptest = 0;
> } else
> if (s->nic)
> qemu_send_packet(qemu_get_queue(s->nic), s->buffer,
> s->xmit_pos);
>
> s->csr[0] &= ~0x0008; /* clear TDMD */
> s->csr[4] |= 0x0004; /* set TXSTRT */
> s->xmit_pos = -1;
>
> txdone:
>
> Best regards,
> -Gonglei
>
>
