[Libvirt-cim] [PATCH 2 of 3] [RFC][CU] Add EI CIMXML parser

[Dan Smith]

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 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/20080211/486c5b51/attachment.sig>