[Libvirt-cim] [PATCH 1 of 6] ComputerSystem: enhance enum functionality for use in assocs

[Jay Gagnon]

Heidi Eckhart wrote:
> # HG changeset patch
> # User Heidi Eckhart <heidieck at linux.vnet.ibm.com>
> # Date 1204541457 -3600
> # Node ID 7a93b0ab4de3663e75f43176cce67b70e2064fd7
> # Parent  27e78288c82426835126eaa07dba1e425c756f3f
> ComputerSystem: enhance enum functionality for use in assocs
> Signed-off-by: Heidi Eckhart <heidieck at linux.vnet.ibm.com>
>
>
> +CMPIStatus enum_domains(const CMPIBroker *broker,
> +                        const CMPIObjectPath *reference,
> +                        struct inst_list *instlist)
> +{
> +        CMPIStatus s = {CMPI_RC_OK, NULL};
>          virDomainPtr *list = NULL;
> +        virConnectPtr conn = NULL;
>          int count;
>          int i;
>
> +        conn = connect_by_classname(broker, CLASSNAME(reference), &s);
> +        if (conn == NULL)
> +                goto out;
> +
>          count = get_domain_list(conn, &list);
> -        if (count <= 0)
> -                goto out;
> +        if (count <= 0) {
> +                cu_statusf(broker, &s,
> +                           CMPI_RC_ERR_FAILED,
> +                           "Failed to get domain list");
> +                goto out;
> +        }
>
>          for (i = 0; i < count; i++) {
> -                CMPIInstance *inst;
> -
> -                inst = get_typed_instance(broker,
> -                                          pfx_from_conn(conn),
> -                                          "ComputerSystem",
> -                                          ns);
> -                if (inst == NULL)
> +                CMPIInstance *inst = NULL;
> +                
> +                s = instance_from_dom(broker,
> +                                      reference,
> +                                      conn,
> +                                      list[i],  
> +                                      &inst);
> +                if (s.rc != CMPI_RC_OK)
>                          goto end;
>
> -                if (instance_from_dom(broker, 
> -                                      list[i], 
> -                                      pfx_from_conn(conn), 
> -                                      inst))
> -                        inst_list_add(instlist, inst);
> -
> -        end:
> +                inst_list_add(instlist, inst);
> +
> +          end:
>                  virDomainFree(list[i]);
>          }
> - out:
> +
> + out:
> +        virConnectClose(conn);
>          free(list);
>
> -        return 1;
> +        return s;
>  }
>
>   
This isn't so much a problem with the patch as it is something brought 
to my attention by the patch, but I'm not a huge fan of the "goto end;" 
bit in the for loop.  I know we use "goto out;" a ton for error cases, 
which I've gotten used to, but I'm still not a big fan of using goto and 
in this case I think we could avoid it without much trouble.  Something 
like:

if (s.rc != CMPI_RC_OK) {
        virDomainFree(list[i]);
        continue;
}

should achieve the same functionality but in what I would call a much 
cleaner way.  Everyone, feel free to chime in here one way or the other.

-- 

-Jay