[libvirt] [PATCH 2/2] python: Expose binding for virNodeGetMemoryStats()
Peter Krempa
pkrempa at redhat.com
Fri Dec 2 23:21:19 UTC 2011
Dňa 2.12.2011 18:23, Eric Blake wrote / napísal(a):
> On 11/28/2011 10:19 AM, Peter Krempa wrote:
>> + if (!(ret = PyDict_New())) {
>> + free(stats);
>> + return VIR_PY_NONE;
>> + }
>> + for (i = 0; i< nparams; i++) {
>> + PyDict_SetItem(ret,
>> + libvirt_constcharPtrWrap(stats[i].field),
>> + libvirt_ulonglongWrap(stats[i].value));
>> + }
>
> Copy and paste, so not a problem with this patch any more so than the
> other functions that used the same code pattern, but can PyDict_SetItem
> fail? If so, should be be reclaiming the entries added so far before
> returning overall failure, instead of silently truncating the return
> dictionary by omitting the entries that weren't inserted properly?
>
Well, I was wondering myself why there's no check for insertion of the
item into the dictionary as it is apparently allocating (tons of)
memory. I was following the pattern used in already existing code.
The better approach would be:
+ for (i = 0; i< nparams; i++) {
+ if (PyDict_SetItem(ret,
+ libvirt_constcharPtrWrap(stats[i].field),
+ libvirt_ulonglongWrap(stats[i].value))<0){ +
Py_XDECREF(ret);
+ return VIR_PY_NONE;
+ }
+ }
Free the reference and return an error. Maybe it would be useful to
cause an exception, but I'm not a python binding guru. Should I make it
more robust? Or maybe we should clean this up later in all instances of
improper dictionary insertions?
Peter
More information about the libvir-list
mailing list