From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40853) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XrLzk-0000qY-Lz for qemu-devel@nongnu.org; Thu, 20 Nov 2014 02:17:41 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XrLzf-0006Bl-Qa for qemu-devel@nongnu.org; Thu, 20 Nov 2014 02:17:36 -0500 Received: from szxga02-in.huawei.com ([119.145.14.65]:40001) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XrLzf-00069s-04 for qemu-devel@nongnu.org; Thu, 20 Nov 2014 02:17:31 -0500 Message-ID: <546D9449.5020107@huawei.com> Date: Thu, 20 Nov 2014 15:12:09 +0800 From: Gonglei 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> <546D905A.5020306@redhat.com> In-Reply-To: <546D905A.5020306@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: Jason Wang Cc: Paolo Bonzini , "qemu-devel@nongnu.org" , "stefanha@redhat.com" , "Huangpeng (Peter)" On 2014/11/20 14:55, Jason Wang wrote: > 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 >> Indeed. Your approach is better. :) >> 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, Yes. the same reason with the original issue. >> 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) More complicated, because we can't initialize iov when "size < ETHER_ADDR_LEN * 2". Best regards, -Gonglei