5:50 p.m.
A reposting of a patch into 3 separate patches for each of the Coverity
issues found from commit id 'cc0e8c24'. The changes were already ACK'd,
but I figured I'd repost anyway.
v1:
http://www.redhat.com/archives/libvir-list/2014-October/msg01012.html
John Ferlan (3):
virnetdev: Resolve Coverity DEADCODE
virnetdev: Resolve Coverity FORWARD_NULL
virnetdev: Resolve Coverity RESOURCE_LEAK
src/util/virnetdev.c | 38 +++++++++++++++++++++-----------------
1 file changed, 21 insertions(+), 17 deletions(-)
--
1.9.3
5:50 p.m.
New subject: [libvirt] [PATCH v2 1/3] virnetdev: Resolve Coverity DEADCODE
Coverity complains that because the for loop is from 0 to 5 (max tokens)
and the impending switch/case statements used each of the #define values
that the 'default' wouldn't reachable. This patch will convert the
#define's
into enum's and add the obligatory dead_error_begin marker for these type
situations.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/util/virnetdev.c | 34 ++++++++++++++++++++--------------
1 file changed, 20 insertions(+), 14 deletions(-)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 127bfb2..36e84d7 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -59,15 +59,19 @@ VIR_LOG_INIT("util.netdev");
#define PROC_NET_DEV_MCAST "/proc/net/dev_mcast"
#define MAX_MCAST_SIZE 50*14336
#define VIR_MCAST_NAME_LEN (IFNAMSIZ + 1)
-#define VIR_MCAST_INDEX_TOKEN_IDX 0
-#define VIR_MCAST_NAME_TOKEN_IDX 1
-#define VIR_MCAST_USERS_TOKEN_IDX 2
-#define VIR_MCAST_GLOBAL_TOKEN_IDX 3
-#define VIR_MCAST_ADDR_TOKEN_IDX 4
-#define VIR_MCAST_NUM_TOKENS 5
#define VIR_MCAST_TOKEN_DELIMS " \n"
#define VIR_MCAST_ADDR_LEN (VIR_MAC_HEXLEN + 1)
+typedef enum {
+ VIR_MCAST_TYPE_INDEX_TOKEN,
+ VIR_MCAST_TYPE_NAME_TOKEN,
+ VIR_MCAST_TYPE_USERS_TOKEN,
+ VIR_MCAST_TYPE_GLOBAL_TOKEN,
+ VIR_MCAST_TYPE_ADDR_TOKEN,
+
+ VIR_MCAST_TYPE_LAST
+} virMCastType;
+
typedef struct _virNetDevMcastEntry virNetDevMcastEntry;
typedef virNetDevMcastEntry *virNetDevMcastEntryPtr;
struct _virNetDevMcastEntry {
@@ -2076,7 +2080,7 @@ static int virNetDevParseMcast(char *buf, virNetDevMcastEntryPtr
mcast)
char *saveptr;
char *endptr;
- for (ifindex = 0, next = buf; ifindex < VIR_MCAST_NUM_TOKENS; ifindex++,
+ for (ifindex = 0, next = buf; ifindex < VIR_MCAST_TYPE_LAST; ifindex++,
next = NULL) {
token = strtok_r(next, VIR_MCAST_TOKEN_DELIMS, &saveptr);
@@ -2087,8 +2091,8 @@ static int virNetDevParseMcast(char *buf, virNetDevMcastEntryPtr
mcast)
return -1;
}
- switch (ifindex) {
- case VIR_MCAST_INDEX_TOKEN_IDX:
+ switch ((virMCastType)ifindex) {
+ case VIR_MCAST_TYPE_INDEX_TOKEN:
if (virStrToLong_i(token, &endptr, 10, &num) < 0) {
virReportSystemError(EINVAL,
_("Failed to parse interface index from
'%s'"),
@@ -2098,7 +2102,7 @@ static int virNetDevParseMcast(char *buf, virNetDevMcastEntryPtr
mcast)
}
mcast->index = num;
break;
- case VIR_MCAST_NAME_TOKEN_IDX:
+ case VIR_MCAST_TYPE_NAME_TOKEN:
if (virStrncpy(mcast->name, token, strlen(token),
VIR_MCAST_NAME_LEN) == NULL) {
virReportSystemError(EINVAL,
@@ -2107,7 +2111,7 @@ static int virNetDevParseMcast(char *buf, virNetDevMcastEntryPtr
mcast)
return -1;
}
break;
- case VIR_MCAST_USERS_TOKEN_IDX:
+ case VIR_MCAST_TYPE_USERS_TOKEN:
if (virStrToLong_i(token, &endptr, 10, &num) < 0) {
virReportSystemError(EINVAL,
_("Failed to parse users from
'%s'"),
@@ -2117,7 +2121,7 @@ static int virNetDevParseMcast(char *buf, virNetDevMcastEntryPtr
mcast)
}
mcast->users = num;
break;
- case VIR_MCAST_GLOBAL_TOKEN_IDX:
+ case VIR_MCAST_TYPE_GLOBAL_TOKEN:
if (virStrToLong_i(token, &endptr, 10, &num) < 0) {
virReportSystemError(EINVAL,
_("Failed to parse users from
'%s'"),
@@ -2127,7 +2131,7 @@ static int virNetDevParseMcast(char *buf, virNetDevMcastEntryPtr
mcast)
}
mcast->global = num;
break;
- case VIR_MCAST_ADDR_TOKEN_IDX:
+ case VIR_MCAST_TYPE_ADDR_TOKEN:
if (virMacAddrParseHex((const char*)token,
&mcast->macaddr) < 0) {
virReportSystemError(EINVAL,
@@ -2135,7 +2139,9 @@ static int virNetDevParseMcast(char *buf, virNetDevMcastEntryPtr
mcast)
buf);
}
break;
- default:
+
+ /* coverity[dead_error_begin] */
+ case VIR_MCAST_TYPE_LAST:
break;
}
}
--
1.9.3
5:50 p.m.
New subject: [libvirt] [PATCH v2 2/3] virnetdev: Resolve Coverity FORWARD_NULL
The complaint is that if cleanup is called when virFileReadAll fails,
then mcast->entries is NULL and could be dereferenced in the clear
function. After following the code some - I saw that the caller to
the function (virNetDevGetMulticastTable) will also call
virNetDevMcastListClear if this function returns -1, so this
isn't necessary, so I removed the call.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/util/virnetdev.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 36e84d7..4fea4cb 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -2200,9 +2200,6 @@ static int virNetDevGetMcastList(const char *ifname,
ret = 0;
cleanup:
- if (ret < 0)
- virNetDevMcastListClear(mcast);
-
VIR_FREE(entry);
return ret;
--
1.9.3
5:50 p.m.
New subject: [libvirt] [PATCH v2 3/3] virnetdev: Resolve Coverity RESOURCE_LEAK
virFileReadAll returns a chunk of memory that needs to be free'd when
done
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/util/virnetdev.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 4fea4cb..0c9c1f9 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -2200,6 +2200,7 @@ static int virNetDevGetMcastList(const char *ifname,
ret = 0;
cleanup:
+ VIR_FREE(buf);
VIR_FREE(entry);
return ret;
--
1.9.3
6:21 p.m.
On 10/29/2014 04:50 PM, John Ferlan wrote:
A reposting of a patch into 3 separate patches for each of the
Coverity
issues found from commit id 'cc0e8c24'. The changes were already ACK'd,
but I figured I'd repost anyway.
Thanks; looks better, and good for 1.2.10.
v1:
http://www.redhat.com/archives/libvir-list/2014-October/msg01012.html
John Ferlan (3):
virnetdev: Resolve Coverity DEADCODE
virnetdev: Resolve Coverity FORWARD_NULL
virnetdev: Resolve Coverity RESOURCE_LEAK
src/util/virnetdev.c | 38 +++++++++++++++++++++-----------------
1 file changed, 21 insertions(+), 17 deletions(-)
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
3662
days inactive
3662
days old
4 comments
2 participants
participants (2)
-
Eric Blake -
John Ferlan