[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]