From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33304) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XrLFP-0005cT-OJ for qemu-devel@nongnu.org; Thu, 20 Nov 2014 01:29:48 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XrLFK-0006x4-U9 for qemu-devel@nongnu.org; Thu, 20 Nov 2014 01:29:43 -0500 Received: from mx1.redhat.com ([209.132.183.28]:54819) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XrLFK-0006wy-Ls for qemu-devel@nongnu.org; Thu, 20 Nov 2014 01:29:38 -0500 Message-ID: <546D8A48.8040502@redhat.com> Date: Thu, 20 Nov 2014 07:29:28 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <1416463034-8264-1-git-send-email-arei.gonglei@huawei.com> <1416463034-8264-5-git-send-email-arei.gonglei@huawei.com> In-Reply-To: <1416463034-8264-5-git-send-email-arei.gonglei@huawei.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH 4/4] rtl8139: fix Pointer to local outside scope List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: arei.gonglei@huawei.com, qemu-devel@nongnu.org Cc: peter.huangpeng@huawei.com, stefanha@redhat.com On 20/11/2014 06:57, arei.gonglei@huawei.com wrote: > From: Gonglei > > Coverity spot: > Assigning: iov = struct iovec [3]({{buf, 12UL}, > {(void *)dot1q_buf, 4UL}, > {buf + 12, size - 12}}) > (address of temporary variable of type struct iovec [3]). > out_of_scope: Temporary variable of type struct iovec [3] goes out of scope. > > Pointer to local outside scope (RETURN_LOCAL) > use_invalid: > Using iov, which points to an out-of-scope temporary variable of type struct iovec [3]. > > Signed-off-by: Gonglei > --- > hw/net/rtl8139.c | 36 ++++++++++++------------------------ > 1 file changed, 12 insertions(+), 24 deletions(-) > > diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c > index 8b8a1b1..426171b 100644 > --- a/hw/net/rtl8139.c > +++ b/hw/net/rtl8139.c > @@ -1775,6 +1775,8 @@ static void rtl8139_transfer_frame(RTL8139State *s, uint8_t *buf, int size, > int do_interrupt, const uint8_t *dot1q_buf) > { > struct iovec *iov = NULL; > + size_t buf2_size; > + uint8_t *buf2 = NULL; > > if (!size) > { > @@ -1789,35 +1791,21 @@ static void rtl8139_transfer_frame(RTL8139State *s, uint8_t *buf, int size, > { .iov_base = buf + ETHER_ADDR_LEN * 2, > .iov_len = size - ETHER_ADDR_LEN * 2 }, > }; > - } > > - if (TxLoopBack == (s->TxConfig & TxLoopBack)) > - { > - size_t buf2_size; > - uint8_t *buf2; > - > - if (iov) { > - buf2_size = iov_size(iov, 3); > - buf2 = g_malloc(buf2_size); > - iov_to_buf(iov, 3, 0, buf2, buf2_size); > - buf = buf2; > - } > + buf2_size = iov_size(iov, 3); > + buf2 = g_malloc(buf2_size); > + iov_to_buf(iov, 3, 0, buf2, buf2_size); > + buf = buf2; > + } This makes rtl8139 even slower than it is for the vlantag case, using a bounce buffer for every packet. Perhaps another solution could be to do struct iovec *iov = NULL; struct iovec vlan_iov[3]; ... if (dot1q_buf && size >= ETHER_ADDR_LEN * 2) { ... memcpy(vlan_iov, &(struct iovec[3]) { ... }, sizeof(vlan_iov)); iov = vlan_iov; } (I think "vlan_iov = (struct iovec[3]) { ... };" does not work, but I may be wrong). Stefan, what do you think? Paolo > > + if (TxLoopBack == (s->TxConfig & TxLoopBack)) { > DPRINTF("+++ transmit loopback mode\n"); > rtl8139_do_receive(qemu_get_queue(s->nic), buf, size, do_interrupt); > - > - if (iov) { > - g_free(buf2); > - } > - } > - else > - { > - if (iov) { > - qemu_sendv_packet(qemu_get_queue(s->nic), iov, 3); > - } else { > - qemu_send_packet(qemu_get_queue(s->nic), buf, size); > - } > + } else { > + qemu_send_packet(qemu_get_queue(s->nic), buf, size); > } > + > + g_free(buf2); > } > > static int rtl8139_transmit_one(RTL8139State *s, int descriptor) >