On 10/29/2014 03:28 PM, John Ferlan wrote:
> Coverity discovered a few issues with recent changes in this module
> from commit id 'cc0e8c24'. This patches fixes those.
Might be nice to summarize what those errors were.
>
> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
> ---
> src/util/virnetdev.c | 37 +++++++++++++++++++++----------------
> 1 file changed, 21 insertions(+), 16 deletions(-)
>
> diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
> index 127bfb2..8bd1af4 100644
> --- a/src/util/virnetdev.c
> +++ b/src/util/virnetdev.c
> @@ -59,15 +59,20 @@ 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
Is VIR_MCAST_NUM_TOKENS still used after this patch? Or can you delete
it in favor of...
> #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
...this?
>
> - 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,
> @@ -2135,7 +2140,9 @@ static int virNetDevParseMcast(char *buf, virNetDevMcastEntryPtr mcast)
> buf);
> }
> break;
> - default:
> +
> + /* coverity[dead_error_begin] */
> + case VIR_MCAST_TYPE_LAST:
> break;
Okay, I can see what you did so far (convert from #defines to an enum to
let the compiler warn if we miss any cases, including a way to silence
Coverity about the known-dead branch of the switch).
> }
> }
> @@ -2194,9 +2201,7 @@ static int virNetDevGetMcastList(const char *ifname,
>
> ret = 0;
> cleanup:
> - if (ret < 0)
> - virNetDevMcastListClear(mcast);
> -
> + VIR_FREE(buf);
But this one doesn't have any context in the commit message at why it is
needed. However, in looking at the function itself and the referenced
commit that introduced the problem, I agree with the fix. ACK, although
it might be better to do this as two separate commits.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list