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

[Dan Smith]

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 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/20080111/411eaeec/attachment.sig>