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