From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:50960)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <paolo.bonzini@gmail.com>) id 1XrOaY-00027o-RG
	for qemu-devel@nongnu.org; Thu, 20 Nov 2014 05:03:55 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <paolo.bonzini@gmail.com>) id 1XrOa6-0003lu-MA
	for qemu-devel@nongnu.org; Thu, 20 Nov 2014 05:03:46 -0500
Received: from mail-wi0-x236.google.com ([2a00:1450:400c:c05::236]:45630)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <paolo.bonzini@gmail.com>) id 1XrOa6-0003lV-Fv
	for qemu-devel@nongnu.org; Thu, 20 Nov 2014 05:03:18 -0500
Received: by mail-wi0-f182.google.com with SMTP id h11so4726027wiw.9
	for <qemu-devel@nongnu.org>; Thu, 20 Nov 2014 02:03:17 -0800 (PST)
Sender: Paolo Bonzini <paolo.bonzini@gmail.com>
Message-ID: <546DBC61.8090104@redhat.com>
Date: Thu, 20 Nov 2014 11:03:13 +0100
From: Paolo Bonzini <pbonzini@redhat.com>
MIME-Version: 1.0
References: <1416463034-8264-1-git-send-email-arei.gonglei@huawei.com>	<1416463034-8264-4-git-send-email-arei.gonglei@huawei.com>	<546D8BDA.9080205@redhat.com>
	<546D8DD2.30304@huawei.com>	<546D9380.2050606@redhat.com>
	<546D9A93.5000706@huawei.com>
In-Reply-To: <546D9A93.5000706@huawei.com>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH 3/4] pcnet: fix Negative array index read
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Gonglei <arei.gonglei@huawei.com>
Cc: "qemu-devel@nongnu.org" <qemu-devel@nongnu.org>, "stefanha@redhat.com" <stefanha@redhat.com>, "Huangpeng (Peter)" <peter.huangpeng@huawei.com>



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