On 11/20/2014 02:29 PM, Paolo Bonzini wrote: > > On 20/11/2014 06:57, arei.gong...@huawei.com wrote: >> From: Gonglei <arei.gong...@huawei.com> >> >> 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 <arei.gong...@huawei.com> >> --- >> 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)