Re: [Libvirt-cim] [PATCH 2 of 3] Make alloc_cap_instances() available for external use by EC

HE> Mhh, as we have many free operations now that can handle NULL HE> pointers, I suggest to also make inst_list_free() able to handle HE> NULL pointers. But they're not NULL pointers at that point, the list.list is a garbage pointer. Unless you define them as: struct inst_list list = {NULL, 0, 0}; Otherwise, the pointer will be garbage and a free() will smash the heap. The inst_list_init() call doesn't allocate any memory, it just initializes the contents of the struct, so why not always do that before proceeding? HE> Agreed. I will cook up a patch for inst_list_free(). The intention HE> to move the inst_list_init() functions below HE> connect_by_classname() was to avoid some cycles, because the case HE> where the provider exits right after provider_is_responsible() or HE> connect_by_classname() can happen often. I think this optimization is likely lost in the noise of the roaring CIMOM above us, so I'm not sure it's all that important. However, a macro could help make this cleaner I think. Something like what they do in the kernel might be helpful: #define DECLARE_INST_LIST(x) struct inst_list x = {NULL, 0, 0} so that we can just do this in a function to have pre-initialized lists and avoid the inst_list_init() calls: int function_foo(...) { int a; char b; DECLARE_INST_LIST(list_one); DECLARE_INST_LIST(list_two); ... } I still question the value of this optimization, as I think the compiler will inline the inst_list_init() function. At that point, this: struct inst_list foo = {NULL, 0, 0}; surely becomes the same code as this: struct inst_list foo; foo.list = NULL; foo.max = foo.cur = 0; Anyone else have thoughts on this? HE> Agreed. I saw that we already have a task for "Fix up all the HE> error codes in returned status values (everything is currently HE> CMPI_RC_ERR_FAILED)". So we should leave this discussion for HE> there. I will remove this change. Yes, we definitely have work to do on our return codes and error messages, but I'd rather address them as a group than sprinkle them in with other changes :) Thanks! -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms(a)us.ibm.com