[Libvirt-cim] [PATCH] Set additional attribute values in Virt_ComputerSystem.c

[Dan Smith]

KR> +static int set_other_id_info(const CMPIBroker *broker,
KR> +                             char *uuid,
KR> +                             const char *prefix,
KR> +                             CMPIInstance *instance)
KR> +{
KR> +        CMPIStatus s;
KR> +        CMPIArray *id_info;
KR> +        char *info[3];
KR> +        int count = 3; 
KR> +        char *type = "Virtual System";
KR> +        char *model;
KR> +        int i;
KR> +
KR> +        id_info = CMNewArray(broker,
KR> +                             count,
KR> +                             CMPI_string,
KR> +                             &s);
KR> +
KR> +        if (s.rc != CMPI_RC_OK)
KR> +                return 0;
KR> +
KR> +        if (asprintf(&model, "%s%s", prefix, type) == 1)
KR> +                return 0;
KR> +
KR> +        info[0] = uuid;
KR> +        info[1] = model;
KR> +        info[2] = type;
KR> +
KR> +        for (i = 0; i < count; i++) {
KR> +                CMPIString *tmp = CMNewString(broker, info[i], NULL);
KR> +                CMSetArrayElementAt(id_info, i,
KR> +                                    &tmp,
KR> +                                    CMPI_string);
KR> +        }
KR> +
KR> +        CMSetProperty(instance, "OtherIdentifyingInfo",
KR> +                      &id_info, CMPI_stringA);
KR> +
KR> +        return 1;
KR> +}

This is a style thing, I guess, but wouldn't it be easier to combine
this function with the one that sets the descriptions?  You could loop
through both string arrays at the same time and construct the two
CMPIArrays together.  The two functions are almost identical, so I
think doing them together would add six lines to the above function
instead of the 30 it takes for the separate function.

KR>  static int instance_from_dom(const CMPIBroker *broker,
KR>                               virDomainPtr dom,
KR> +                             const char *prefix,
KR>                               CMPIInstance *instance)
KR>  {
KR> +        char *uuid;
KR> +
KR>          if (!set_name_from_dom(dom, instance)) {
KR>                  /* Print trace error */
KR>                  return 0;
KR>          }

KR> -        if (!set_uuid_from_dom(dom, instance)) {
KR> +        if (!set_uuid_from_dom(dom, instance, &uuid)) {
KR>                  /* Print trace error */
KR>                  return 0;
KR>          }
KR> @@ -269,7 +347,20 @@ static int instance_from_dom(const CMPIB
KR>                  return 0;

Leak UUID here...

KR>          }

KR> +        if (!set_other_id_info(broker, uuid, prefix, instance)) {
KR> +                /* Print trace error */
KR> +                free(uuid);
KR> +                return 0;

...but not here...

KR> +        }
KR> +
KR> +        if (!set_id_desc(broker, instance)) {
KR> +                /* Print trace error */
KR> +                return 0;

...and then again here.

KR> +        }
KR> +
KR>          /* More attributes here, of course */
KR> +
KR> +        free(uuid);

KR>          return 1;
KR>  }

Can we get either an "err:" target or a return variable and an "out:"
target please? :)

-- 
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms at us.ibm.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 188 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvirt-cim/attachments/20080204/f9283830/attachment.sig>