[libvirt] [PATCH 0/2] Two Coverity Fixes (thanks John!)
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
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
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
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@redhat.com> (series) John
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
participants (3)
-
John Ferlan -
Ján Tomko -
Laine Stump