_speedups' glitch on Markup.escape(None, False) call

[cboos]

This problem was detected in ​#T10292 and tracked down to a call to Markup.escape(None, False) by trbs.

The "normal" behavior for such a call is:

genshi/tests/core.py

@@ -86 +86 @@
         assert type(markup) is Markup
         self.assertEquals(string, unescape(markup))
 
+    def test_Markup_escape_None_noquotes(self):
+        markup = Markup.escape(None, False)
+        assert type(markup) is Markup
+        self.assertEquals('', markup)
+
     def test_add_str(self):
         markup = Markup('<b>foo</b>') + '<br/>'
         assert type(markup) is Markup

But when the _speedups are present, that test fails with:

# PYTHONPATH=. python genshi/tests/core.py
......E........................................
======================================================================
ERROR: test_Markup_escape_None_noquotes (__main__.MarkupTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "genshi/tests/core.py", line 90, in test_Markup_escape_None_noquotes
    markup = Markup.escape(None, False)
TypeError: unicode() argument 2 must be string, not bool

The following change fixes the problem:

genshi/_speedups.c

@@ -194 +194 @@
         return NULL;
     }
     if (PyObject_Not(text)) {
-        return type->tp_new(type, args, NULL);
+        return type->tp_new(type, PyTuple_New(0), NULL);
     }
     if (PyObject_TypeCheck(text, type)) {
         Py_INCREF(text);

I'm not completely sure if we can get away with not doing a DECREF on that 0-tuple, so perhaps a safer fix would be:

genshi/_speedups.c

@@ -194 +194 @@
         return NULL;
     }
     if (PyObject_Not(text)) {
-        return type->tp_new(type, args, NULL);
+        args = PyTuple_New(0);
+        text = type->tp_new(type, args, NULL);
+        Py_DECREF(args);
+        return text;
     }
     if (PyObject_TypeCheck(text, type)) {
         Py_INCREF(text);

Your take ;-)

[cboos]

applies on 0.6.x and trunk

[hodgestar]

Fix with check for failure allocated empty tuple.

[hodgestar]

I think we do need to DECREF the tuple and I think we also need to check that PyTuple_New doesn't fail. Updated patch attached. If you (cboos) agree I'll commit.

[cboos]

I was under the impression that PyTuple_New(0) could never fail, but looking closer at the code in ​tupleobject.c it seems there could be pathological cases when this could happen (e.g. no zero tuple allocated so far and in an out of memory condition), so your change is indeed safer.

[hodgestar]

Patch committed in r1168 and backported to 0.6.x in r1169. Thank again cboos.