[Libvirt-cim] [PATCH 1/6] VSMS: Coverity cleanups

[Boris Fiuczynski]

On 01/22/2014 08:30 PM, John Ferlan wrote:
> A new version of Coverity found a number of issues:
>
> parse_ip_address(): FORWARD_NULL
>    - Benign issue regarding how 'tmp_ip' was compared against NULL for
>      the IPv6 processing and then used blindly later when strdup()'ing
>      into *ip. Rather than use NULL check, compare against return of 1
>      or more which indicates that something is there
>
> update_system_settings(): RESOURCE_LEAK
>    - The 'uuid' value was being leaked if strdup()'d.  Also rather than
>      strdup()'g and strdup()'d value and risking failure, just assign the
>      initially strdup()'d value and reinitialize uuid to NULL
>
> fv_vssd_to_domain(): USE_AFTER_FREE
>    - The domain->os_info.fv.arch is free()'d only to be potentially
>      strdup()'d after processing the 'cu_get_str_prop()' for "Arch".
>      The complaint was that it was possible to not strdup() a new value
>      and thus possible to pass a free()'d value to get_default_machine().
>      Passing a NULL is not an issue as that is checked.
>
>      Additionally found by inspection, 'val' was not initialized to NULL,
>      so the setting of os_info.fv.arch may not be what was expected. Also,
>      after processing "Arch" it was not reinitialized to NULL so its
>      contents could potentially have been saved in os_info.fv.machine.
>
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>   src/Virt_VirtualSystemManagementService.c | 11 ++++++++---
>   1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/src/Virt_VirtualSystemManagementService.c b/src/Virt_VirtualSystemManagementService.c
> index 5c7238f..b624d8c 100644
> --- a/src/Virt_VirtualSystemManagementService.c
> +++ b/src/Virt_VirtualSystemManagementService.c
> @@ -464,7 +464,7 @@ static int fv_vssd_to_domain(CMPIInstance *inst,
>   {
>           int ret = 1;
>           int retr;
> -        const char *val;
> +        const char *val = NULL;
>           const char *domtype = NULL;
>           const char *ostype = "hvm";
>           struct capabilities *capsinfo = NULL;
> @@ -494,6 +494,7 @@ static int fv_vssd_to_domain(CMPIInstance *inst,
>           }
>
>           free(domain->os_info.fv.arch);
> +        domain->os_info.fv.arch = NULL;
>           retr = cu_get_str_prop(inst, "Arch", &val);
>           if (retr != CMPI_RC_OK) {
>                   if (capsinfo != NULL) { /* set default */
> @@ -506,6 +507,8 @@ static int fv_vssd_to_domain(CMPIInstance *inst,
>                   domain->os_info.fv.arch = strdup(val);
>
>           free(domain->os_info.fv.machine);
> +        domain->os_info.fv.machine = NULL;
> +        val = NULL;
>           retr = cu_get_str_prop(inst, "Machine", &val);
>           if (retr != CMPI_RC_OK) {
>                   if (capsinfo != NULL && domtype != NULL) { /* set default */
> @@ -1415,7 +1418,7 @@ static int parse_ip_address(const char *id,
>           if (strstr(id, "[") != NULL) {
>                   /* its an ipv6 address */
>                   ret = sscanf(id, "%a[^]]]:%as",  &tmp_ip, &tmp_port);
> -                if (tmp_ip != NULL) {
> +                if (ret >= 1) {
>                           tmp_ip = realloc(tmp_ip, strlen(tmp_ip) + 2);
>                           if (tmp_ip == NULL) {
>                                   ret = 0;
> @@ -2798,7 +2801,8 @@ static CMPIStatus update_system_settings(const CMPIContext *context,
>           }
>
>           if ((dominfo->uuid == NULL) || (STREQ(dominfo->uuid, ""))) {
> -                dominfo->uuid = strdup(uuid);
> +                dominfo->uuid = uuid;
> +                uuid = NULL;
I am getting a compile error here and below for the free of uuid.
error: assignment discards 'const' qualifier from pointer target type 
[-Werror]
error: passing argument 1 of 'free' discards 'const' qualifier from 
pointer target type [-Werror]

Removing the const in the declaration works... for me.

>           } else if (!STREQ(uuid, dominfo->uuid)) {
>                   cu_statusf(_BROKER, &s,
>                              CMPI_RC_ERR_FAILED,
> @@ -2829,6 +2833,7 @@ static CMPIStatus update_system_settings(const CMPIContext *context,
>           }
>
>    out:
> +        free(uuid);
>           free(xml);
>           virDomainFree(dom);
>           virConnectClose(conn);
>


-- 
Mit freundlichen Grüßen/Kind regards
    Boris Fiuczynski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martina Köderitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294