Re: [PATCH] two small patches for the hippo python bindings



On Tue, 2008-10-21 at 14:15 +0200, Tomeu Vizoso wrote:
> Hi,
> 
> needed to access hippo_canvas_box_sort() from python and also found a
> crasher when hippo_canvas_box_find_box_child() returned NULL.

Couple of minor problems; I know these aren't all new problems, some are
problems in existing code that you are moving about.

+    if (box_child)
+        return py_hippo_canvas_box_child_new(box_child);
+    else
+        return Py_None;

When returning one of the constants, you always need to increase the
refcount first. So:

	Py_INCREF(Py_None);
	return Py_None;

(There is a handy Py_RETURN_NONE macro, but we don't seem to be using it
elsewhere, so probably better to avoid it for consistency.)

+static int
+marshal_compare_child_func(HippoCanvasItem *a, HippoCanvasItem *b, void
*data)
+{
+    PyObject *py_a, *py_b;
+       PyObject *compare = (PyObject*) data;

This looks like the editor was set to 4-space tabs. Fixed 8-space tabs
are standard for all of GNOME.

+       py_a = pygobject_new(G_OBJECT(a));    

I'd suggest checking the return value here. It can return NULL, though
that is unlikely.

+       int retval = 0;
[...]
+       retval = PyInt_AsLong(retobj);
+        if (retval == -1 && PyErr_Occurred())
+            PyErr_Print();    
[...]
+    return retval;

Would recommend:

  long retval;
[...]
  return retval < 0 ? -1 : (retval == 0 ? 0 : 1);

Otherwise looks good to me.

- Owen




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