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. 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