segfaults when fetching instances about cmpi-bindings HOT 9 CLOSED

You're right about Target_Null vs. Target_Void. get_property_at() already has FIXME comments since it has to distinguish between 'value is CMPI_Null' and 'error' - it currently returns NULL (resp nil) for both cases.

Regarding PyTuple_SetItem I found http://edcjones.tripod.com/refcount.html which says that PyTuple_SetItem will INCREF the argument. This is confusing :-/

from cmpi-bindings.

After looking more closely, I fixed this by defining Target_Null as PyNone for Python. This now also matches the Target_Null_p() predicate definition.

from cmpi-bindings.

From that page that you pasted:

Here is what PyTuple_SetItem(atuple, i, item) does: If atuple[i] currently contains a PyObject, that PyObject is DECREFed. Then atuple[i] is set to item. item is not INCREFed.

If PyTuple_SetItem() fails to insert item, it decrements the reference count for item.
...
Metaphorically, PyTuple_SetItem() grabs responsibility for a reference to item from you. If item is unprotected, PyTuple_SetItem() might DECREF it anyway which can crash Python.

PyDict_SetItem will incref, PyTuple_ and PyDict_ will not. That's how I read the documentation.

Thanks for looking into this!

from cmpi-bindings.

git master should have a proper solution now. Thanks for your help !

from cmpi-bindings.

I'm looking at data_data in swig/cmpi.i. Just trying to clarify something for myself.
I think there is an unnecessary INCREF here:

else if ((dp->type) & CMPI_ARRAY) {
int size = CMGetArrayCount(dp->value.array, NULL);
int i;
result = Target_SizedArray(size);
for (i = 0; i < size; --i) {
CMPIData data = CMGetArrayElementAt(dp->value.array, i, NULL);
Target_Append(result, data_data(&data));
}
Target_INCREF(result);
}

To my knowledge, PyList_New (which is what Target_SizedArray is defined to) will return a "protected" object (a new reference). So the last Target_INCREF will leak memory.

(same problem in data_value)

from cmpi-bindings.

I am even more confused now. Target_SizedArray will return a Python list of the specified size. We shouldn't append to that list (which is what Target_Append does), I think we should use PyList_SetItem. Or you could define an empty list initially and then append to it, although I would expect the former approach to be more memory-efficient.

from cmpi-bindings.

• mibanescu [email protected] [Jun 10. 2011 15:54]:

  I'm looking at data_data in swig/cmpi.i. Just trying to clarify something for myself.
  I think there is an unnecessary INCREF here:

  else if ((dp->type) & CMPI_ARRAY) {
  int size = CMGetArrayCount(dp->value.array, NULL);
  int i;
  result = Target_SizedArray(size);
  for (i = 0; i < size; --i) {
  CMPIData data = CMGetArrayElementAt(dp->value.array, i, NULL);
  Target_Append(result, data_data(&data));
  }
  Target_INCREF(result);
  }

  To my knowledge, PyList_New (which is what Target_SizedArray is defined to) will return a "protected" object (a new reference). So the last Target_INCREF will leak memory.

Yes, you're right. (http://edcjones.tripod.com/refcount.html confirms
this)

Klaus

SUSE LINUX Products GmbH, GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer, HRB 16746 (AG Nürnberg)
Maxfeldstraße 5, 90409 Nürnberg, Germany

from cmpi-bindings.

• mibanescu [email protected] [Jun 10. 2011 15:59]:

I am even more confused now. Target_SizedArray will return a Python
list of the specified size. We shouldn't append to that list (which is
what Target_Append does), I think we should use PyList_SetItem. Or you
could define an empty list initially and then append to it, although I
would expect the former approach to be more memory-efficient.

Yes, indeed. I'll change this to PyList_SetItem.

Klaus

SUSE LINUX Products GmbH, GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer, HRB 16746 (AG Nürnberg)
Maxfeldstraße 5, 90409 Nürnberg, Germany

from cmpi-bindings.

All fixed in version 0.4.16

from cmpi-bindings.