KR> Should this be CMPI_null? I know they're equivalent, but it might be KR> good to use CMPI_null for clarity (and the unlikely chance that the KR> value for CMPI_null is changed) Yes, it should. I changed the signature of that function (about 1000 times) and forgot to update that return. KR> Should we use some kind of found flag here? What if val_arr->type KR> never equals XML_ELEMENT_NODE. Then we pass the last value of KR> val_arr to the function below, even if the element type doesn't KR> equal XML_ELEMENT_NODE. Technically the next call will fail appropriately, but yes, it should be handled better. KR> For these statements above, should we set s.rc to some kind of error KR> return code? Otherwise, we are returning a non-initialized status. Yes :) KR> Just a style note, but for non-boolean variables, aren't we trying to KR> use something like: (iptr != NULL)? Yes :) KR> The return status isn't caught here. parse_instance() could KR> return something other than CMPI_RC_OK if CMNewObjectPath() or KR> CMNewInstance() fail. Yep, that's broken indeed. KR> I know this just and RFC, but don't forget to remove printfs or KR> convert them to CU_DEBUGs. =) Gah! Every...Single...Time... :) Thanks! -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com