Re: [Qemu-devel] [PATCH 4/4] rtl8139: fix Pointer to local outside scope
On 20/11/2014 09:24, Jason Wang wrote: > On 11/20/2014 04:18 PM, Gonglei wrote: >> On 2014/11/20 16:11, Jason Wang wrote: >> >>> On 11/20/2014 04:05 PM, Gonglei wrote: On 2014/11/20 15:50, Jason Wang wrote: >>> 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 >>> > Probably not: you can just do something like: > > if (dot1q_buf && size < ETHER_ADDR_LEN * 2) { > dot1q_buf = NULL; > } > > and check dot1q_buf afterwards. Or just drop the packet since its size > was less than mininum frame length that Ethernet allows. Sorry, I don't understand. But, what's your meaning "initialize iov unconditionally at the beginning"? >>> Something like: >>> >>> @@ -1774,7 +1774,12 @@ static uint32_t >>> rtl8139_RxConfig_read(RTL8139State *s) >>> static void rtl8139_transfer_frame(RTL8139State *s, uint8_t *buf, int size, >>> int do_interrupt, const uint8_t *dot1q_buf) >>> { >>> -struct iovec *iov = NULL; >>> +struct iovec iov[3] = { >>> +{ .iov_base = buf, .iov_len = ETHER_ADDR_LEN * 2 }, >>> +{ .iov_base = (void *) dot1q_buf, .iov_len = VLAN_HLEN }, >>> +{ .iov_base = buf + ETHER_ADDR_LEN * 2, >>> + .iov_len = size - ETHER_ADDR_LEN * 2 }, >>> +}; >>> >>> and assign dot1q_buf to NULL is size is not ok. >>> >> If "size < ETHER_ADDR_LEN * 2", .iov_len = size - ETHER_ADDR_LEN * 2 will be >> negative value; >> and if dot1q_buf is NULL, .iov_base = (void *) dot1q_buf will be NULL too. >> Any side-effect? > > Then you need check dot1q_buf instead of iov after. Iov won't be used if > dot1q_buf is NULL. Or just ignore the rules and use an initializer in the middle of the code: if (size < ETHER_ADDR_LEN * 2) { dot1q_buf = NULL; } struct iovec iov[3] = { ... }; and then check dot1q_buf instead of iov. Plenty of choices, choose your favorite. Paolo
Re: [Qemu-devel] [PATCH 4/4] rtl8139: fix Pointer to local outside scope
On 2014/11/20 16:24, Jason Wang wrote: > On 11/20/2014 04:18 PM, Gonglei wrote: >> On 2014/11/20 16:11, Jason Wang wrote: >> >>> On 11/20/2014 04:05 PM, Gonglei wrote: On 2014/11/20 15:50, Jason Wang wrote: >>> 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 >>> > Probably not: you can just do something like: > > if (dot1q_buf && size < ETHER_ADDR_LEN * 2) { > dot1q_buf = NULL; > } > > and check dot1q_buf afterwards. Or just drop the packet since its size > was less than mininum frame length that Ethernet allows. Sorry, I don't understand. But, what's your meaning "initialize iov unconditionally at the beginning"? >>> Something like: >>> >>> @@ -1774,7 +1774,12 @@ static uint32_t >>> rtl8139_RxConfig_read(RTL8139State *s) >>> static void rtl8139_transfer_frame(RTL8139State *s, uint8_t *buf, int size, >>> int do_interrupt, const uint8_t *dot1q_buf) >>> { >>> -struct iovec *iov = NULL; >>> +struct iovec iov[3] = { >>> +{ .iov_base = buf, .iov_len = ETHER_ADDR_LEN * 2 }, >>> +{ .iov_base = (void *) dot1q_buf, .iov_len = VLAN_HLEN }, >>> +{ .iov_base = buf + ETHER_ADDR_LEN * 2, >>> + .iov_len = size - ETHER_ADDR_LEN * 2 }, >>> +}; >>> >>> and assign dot1q_buf to NULL is size is not ok. >>> >> If "size < ETHER_ADDR_LEN * 2", .iov_len = size - ETHER_ADDR_LEN * 2 will be >> negative value; >> and if dot1q_buf is NULL, .iov_base = (void *) dot1q_buf will be NULL too. >> Any side-effect? > > Then you need check dot1q_buf instead of iov after. Iov won't be used if > dot1q_buf is NULL. >> But that's hacking IMHO. Let's don't do this. ;) >>> Just a suggestion, your call. >> Thanks, Jason :) >> >> Best regards, >> -Gonglei >> >
Re: [Qemu-devel] [PATCH 4/4] rtl8139: fix Pointer to local outside scope
On 11/20/2014 04:18 PM, Gonglei wrote: > On 2014/11/20 16:11, Jason Wang wrote: > >> On 11/20/2014 04:05 PM, Gonglei wrote: >>> On 2014/11/20 15:50, Jason Wang wrote: >>> >> 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 >> Probably not: you can just do something like: if (dot1q_buf && size < ETHER_ADDR_LEN * 2) { dot1q_buf = NULL; } and check dot1q_buf afterwards. Or just drop the packet since its size was less than mininum frame length that Ethernet allows. >>> Sorry, I don't understand. But, >>> what's your meaning "initialize iov unconditionally at the beginning"? >> Something like: >> >> @@ -1774,7 +1774,12 @@ static uint32_t >> rtl8139_RxConfig_read(RTL8139State *s) >> static void rtl8139_transfer_frame(RTL8139State *s, uint8_t *buf, int size, >> int do_interrupt, const uint8_t *dot1q_buf) >> { >> -struct iovec *iov = NULL; >> +struct iovec iov[3] = { >> +{ .iov_base = buf, .iov_len = ETHER_ADDR_LEN * 2 }, >> +{ .iov_base = (void *) dot1q_buf, .iov_len = VLAN_HLEN }, >> +{ .iov_base = buf + ETHER_ADDR_LEN * 2, >> + .iov_len = size - ETHER_ADDR_LEN * 2 }, >> +}; >> >> and assign dot1q_buf to NULL is size is not ok. >> > If "size < ETHER_ADDR_LEN * 2", .iov_len = size - ETHER_ADDR_LEN * 2 will be > negative value; > and if dot1q_buf is NULL, .iov_base = (void *) dot1q_buf will be NULL too. > Any side-effect? Then you need check dot1q_buf instead of iov after. Iov won't be used if dot1q_buf is NULL. > >> Just a suggestion, your call. > Thanks, Jason :) > > Best regards, > -Gonglei >
Re: [Qemu-devel] [PATCH 4/4] rtl8139: fix Pointer to local outside scope
On 2014/11/20 16:11, Jason Wang wrote: > On 11/20/2014 04:05 PM, Gonglei wrote: >> On 2014/11/20 15:50, Jason Wang wrote: >> > 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 > >>> Probably not: you can just do something like: >>> >>> if (dot1q_buf && size < ETHER_ADDR_LEN * 2) { >>> dot1q_buf = NULL; >>> } >>> >>> and check dot1q_buf afterwards. Or just drop the packet since its size >>> was less than mininum frame length that Ethernet allows. >> Sorry, I don't understand. But, >> what's your meaning "initialize iov unconditionally at the beginning"? > > Something like: > > @@ -1774,7 +1774,12 @@ static uint32_t > rtl8139_RxConfig_read(RTL8139State *s) > static void rtl8139_transfer_frame(RTL8139State *s, uint8_t *buf, int size, > int do_interrupt, const uint8_t *dot1q_buf) > { > -struct iovec *iov = NULL; > +struct iovec iov[3] = { > +{ .iov_base = buf, .iov_len = ETHER_ADDR_LEN * 2 }, > +{ .iov_base = (void *) dot1q_buf, .iov_len = VLAN_HLEN }, > +{ .iov_base = buf + ETHER_ADDR_LEN * 2, > + .iov_len = size - ETHER_ADDR_LEN * 2 }, > +}; > > and assign dot1q_buf to NULL is size is not ok. > If "size < ETHER_ADDR_LEN * 2", .iov_len = size - ETHER_ADDR_LEN * 2 will be negative value; and if dot1q_buf is NULL, .iov_base = (void *) dot1q_buf will be NULL too. Any side-effect? > Just a suggestion, your call. Thanks, Jason :) Best regards, -Gonglei
Re: [Qemu-devel] [PATCH 4/4] rtl8139: fix Pointer to local outside scope
On 11/20/2014 04:05 PM, Gonglei wrote: > On 2014/11/20 15:50, Jason Wang wrote: > 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 >> Probably not: you can just do something like: >> >> if (dot1q_buf && size < ETHER_ADDR_LEN * 2) { >> dot1q_buf = NULL; >> } >> >> and check dot1q_buf afterwards. Or just drop the packet since its size >> was less than mininum frame length that Ethernet allows. > Sorry, I don't understand. But, > what's your meaning "initialize iov unconditionally at the beginning"? Something like: @@ -1774,7 +1774,12 @@ static uint32_t rtl8139_RxConfig_read(RTL8139State *s) static void rtl8139_transfer_frame(RTL8139State *s, uint8_t *buf, int size, int do_interrupt, const uint8_t *dot1q_buf) { -struct iovec *iov = NULL; +struct iovec iov[3] = { +{ .iov_base = buf, .iov_len = ETHER_ADDR_LEN * 2 }, +{ .iov_base = (void *) dot1q_buf, .iov_len = VLAN_HLEN }, +{ .iov_base = buf + ETHER_ADDR_LEN * 2, + .iov_len = size - ETHER_ADDR_LEN * 2 }, +}; and assign dot1q_buf to NULL is size is not ok. Just a suggestion, your call.
Re: [Qemu-devel] [PATCH 4/4] rtl8139: fix Pointer to local outside scope
On 2014/11/20 15:50, Jason Wang wrote: >>> 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 >> > > Probably not: you can just do something like: > > if (dot1q_buf && size < ETHER_ADDR_LEN * 2) { > dot1q_buf = NULL; > } > > and check dot1q_buf afterwards. Or just drop the packet since its size > was less than mininum frame length that Ethernet allows. Sorry, I don't understand. But, what's your meaning "initialize iov unconditionally at the beginning"? Best regards, -Gonglei
Re: [Qemu-devel] [PATCH 4/4] rtl8139: fix Pointer to local outside scope
On 11/20/2014 03:12 PM, Gonglei wrote: > 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.gong...@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 > Probably not: you can just do something like: if (dot1q_buf && size < ETHER_ADDR_LEN * 2) { dot1q_buf = NULL; } and check dot1q_buf afterwards. Or just drop the packet since its size was less than mininum frame length that Ethernet allows.
Re: [Qemu-devel] [PATCH 4/4] rtl8139: fix Pointer to local outside scope
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.gong...@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
Re: [Qemu-devel] [PATCH 4/4] rtl8139: fix Pointer to local outside scope
On 11/20/2014 02:29 PM, Paolo Bonzini wrote: > > On 20/11/2014 06:57, arei.gong...@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)
Re: [Qemu-devel] [PATCH 4/4] rtl8139: fix Pointer to local outside scope
On 20/11/2014 06:57, arei.gong...@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) >
[Qemu-devel] [PATCH 4/4] rtl8139: fix Pointer to local outside scope
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; +} +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) -- 1.7.12.4