Re: [libvirt] [PATCH 2/2] python: Expose binding for virNodeGetMemoryStats()
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