From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37332) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XrLeU-0003Fr-0R for qemu-devel@nongnu.org; Thu, 20 Nov 2014 01:55:39 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XrLeQ-0007si-QQ for qemu-devel@nongnu.org; Thu, 20 Nov 2014 01:55:37 -0500 Received: from mx1.redhat.com ([209.132.183.28]:55687) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XrLeQ-0007sC-CX for qemu-devel@nongnu.org; Thu, 20 Nov 2014 01:55:34 -0500 Message-ID: <546D905A.5020306@redhat.com> Date: Thu, 20 Nov 2014 14:55:22 +0800 From: Jason Wang 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> <546D8A48.8040502@redhat.com> In-Reply-To: <546D8A48.8040502@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit 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: Paolo Bonzini , arei.gonglei@huawei.com, qemu-devel@nongnu.org Cc: peter.huangpeng@huawei.com, stefanha@redhat.com On 11/20/2014 02:29 PM, Paolo Bonzini wrote: > > 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 > > Maybe just initialize iov unconditionally at the beginning and check dot1q_buf instead of iov for the rest of the functions. (Need deal with size < ETHER_ADDR_LEN * 2)