10:44 a.m.
John found these two problems with Coverity after I pushed my series
fixing MAC address saving/setting on Mellanox dual port NICs.
Laine Stump (2):
util: fix Coverity complaint (and actual bug) in
virHostdevReadNetConfig()
util: fix Coverity complaint in virNetDevSetNetConfig()
src/util/virhostdev.c | 2 +-
src/util/virnetdev.c | 4 +---
2 files changed, 2 insertions(+), 4 deletions(-)
--
2.13.3
10:44 a.m.
New subject: [libvirt] [PATCH 1/2] util: fix Coverity complaint (and actual bug) in virHostdevReadNetConfig()
Commit 9a94af6d restructured virHostdevReadNetConfig() so that it
would manually set ret = 0 after successfully reading the device's
config, but the "ret = 0" was erroneously placed outside of an "else"
clause, meaning that the the value of ret set in the "if" clause was
unnecessarily and incorrectly overwritten.
This patch moves ret = 0 into the else clause, which should silence
Coverity.
---
src/util/virhostdev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index cca9d81a4..a12224c58 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -641,9 +641,9 @@ virHostdevRestoreNetConfig(virDomainHostdevDefPtr hostdev,
ignore_value(virNetDevSetNetConfig(linkdev, vf,
adminMAC, vlan, MAC, true));
+ ret = 0;
}
- ret = 0;
cleanup:
VIR_FREE(linkdev);
VIR_FREE(MAC);
--
2.13.3
10:44 a.m.
New subject: [libvirt] [PATCH 2/2] util: fix Coverity complaint in virNetDevSetNetConfig()
Commit 81fb440b further qualified an if statement by adding the
boolean saveVlan to the condition, eliminating the need to check
saveVlan in an argument to virAsprintf().
---
src/util/virnetdev.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index ae7da5342..51a6e42c5 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -1936,10 +1936,8 @@ virNetDevSaveNetConfig(const char *linkdev, int vf,
goto cleanup;
/* get admin MAC and vlan tag */
- if (virNetDevGetVfConfig(pfDevName, vf, &oldMAC,
- saveVlan ? &oldVlanTag : NULL) < 0) {
+ if (virNetDevGetVfConfig(pfDevName, vf, &oldMAC, &oldVlanTag) < 0)
goto cleanup;
- }
if (virJSONValueObjectAppendString(configJSON,
VIR_NETDEV_KEYNAME_ADMIN_MAC,
--
2.13.3
12:16 p.m.
On 08/13/2017 11:44 AM, Laine Stump wrote:
John found these two problems with Coverity after I pushed my series
fixing MAC address saving/setting on Mellanox dual port NICs.
Laine Stump (2):
util: fix Coverity complaint (and actual bug) in
virHostdevReadNetConfig()
util: fix Coverity complaint in virNetDevSetNetConfig()
src/util/virhostdev.c | 2 +-
src/util/virnetdev.c | 4 +---
2 files changed, 2 insertions(+), 4 deletions(-)
"Some" will say no need to call out Coverity in the short description,
just somewhere in the commit message a "found by coverity" is
sufficient. And then there's others that are fine with it as is, your
call ;-)
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
(series)
John
1:57 a.m.
On Sun, Aug 13, 2017 at 01:16:13PM -0400, John Ferlan wrote:
On 08/13/2017 11:44 AM, Laine Stump wrote:
> John found these two problems with Coverity after I pushed my series
> fixing MAC address saving/setting on Mellanox dual port NICs.
>
> Laine Stump (2):
> util: fix Coverity complaint (and actual bug) in
> virHostdevReadNetConfig()
> util: fix Coverity complaint in virNetDevSetNetConfig()
>
> src/util/virhostdev.c | 2 +-
> src/util/virnetdev.c | 4 +---
> 2 files changed, 2 insertions(+), 4 deletions(-)
>
"Some" will say no need to call out Coverity in the short description,
just somewhere in the commit message a "found by coverity" is
sufficient. And then there's others that are fine with it as is, your
call ;-)
Yes, 'fix Coverity complaint (and actual bug)' carries pretty much the
same information as 'fix' while wasting way more precious commit summary
characters.
Jan
2657
days inactive
2658
days old
4 comments
3 participants
participants (3)
-
John Ferlan -
Ján Tomko -
Laine Stump