Dan Smith wrote:
+static bool _compare_data(const CMPIData *a, const CMPIData *b) +{ + if (a->type != b->type) + return false; + + if (a->type & CMPI_string) { + const char *as = CMGetCharPtr(a->value.string); + const char *bs = CMGetCharPtr(b->value.string); + + return STREQ(as, bs);
I suggest to use STREQC to become more flexible to the requesters case usage decision.
+ +const struct cu_property *cu_compare_ref(const CMPIObjectPath *ref, + const CMPIInstance *inst, + const struct cu_property *props) +{ + const struct cu_property *p = NULL; + int i; + CMPIStatus s; + + for (i = 0; props[i].name != NULL; i++) { + CMPIData kd, pd; + + p = &props[i]; + + kd = CMGetKey(ref, p->name, &s); + if (s.rc != CMPI_RC_OK) { + if (p->required) + goto out; + else + continue; + }
I fear these lines include a possible crash scenario. For the case that CMGetKey() failed, kd is NULL, but p->required is false - it continues. Later _compare_data() does not check the CMPIData pointer against NULL.
+ + pd = CMGetProperty(inst, p->name, &s); + if (s.rc != CMPI_RC_OK) + goto out; + + if (!_compare_data(&kd, &pd)) + goto out; + } + + p = NULL; + out: + return p; +}
Other than that I have a conceptual question. In this implementation the provider is responsible for defining the list of key properties and has the freedom to mark the properties to be required or not. CIM itself does not give a provider this level of freedom. All key properties of an object path (class) have to be set. And an instance is only then the requested instance, if all key properties fit to the values known to the provider. Otherwise the client requested another instance. The required flag is a very interesting approach, but we should remove it, to avoid misbehavior of providers. A freedom that is not given, can not be used ;). I would also think, that it is not necessary to let the provider define the list of all key properties. As I said - the list of key properties is fix and given with the class definition. The CIMOM checks if all key properties are set and refuses requests with NULL value key properties. A provider does not has to handle this. Now CMPI offers macros to implement a generic approach to iterate over the list of key properties without the knowledge of certain names: CMGetKeyCount() to retrieve the number of key properties iterate over the number of key properties and CMGetKeyAt() to retrieve the key properties at this index Now my suggestion is to implement this generic approach for cu_compare_ref() and remove the props list from the input parameter list. What do you think ? -- Regards Heidi Eckhart Software Engineer Linux Technology Center - Open Hypervisor heidieck@linux.vnet.ibm.com ************************************************** IBM Deutschland Entwicklung GmbH Vorsitzender des Aufsichtsrats: Martin Jetter Geschaeftsfuehrung: Herbert Kircher Sitz der Gesellschaft: Boeblingen Registergericht: Amtsgericht Stuttgart, HRB 243294