From c19488498354cedfec8d4ad3884292d4a5594641 Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Mon, 1 Oct 2012 23:40:37 +0200 Subject: [PATCH] Sanitize and modernize some of the _elementtree code (see issue #16089). --- Modules/_elementtree.c | 166 +++++++++++++++-------------------------- 1 file changed, 59 insertions(+), 107 deletions(-) diff --git a/Modules/_elementtree.c b/Modules/_elementtree.c index 43f9d9b28dc..9e65f423c94 100644 --- a/Modules/_elementtree.c +++ b/Modules/_elementtree.c @@ -123,17 +123,11 @@ deepcopy(PyObject* object, PyObject* memo) return NULL; } - args = PyTuple_New(2); + args = PyTuple_Pack(2, object, memo); if (!args) return NULL; - - Py_INCREF(object); PyTuple_SET_ITEM(args, 0, (PyObject*) object); - Py_INCREF(memo); PyTuple_SET_ITEM(args, 1, (PyObject*) memo); - result = PyObject_CallObject(elementtree_deepcopy_obj, args); - Py_DECREF(args); - return result; } @@ -141,48 +135,16 @@ LOCAL(PyObject*) list_join(PyObject* list) { /* join list elements (destroying the list in the process) */ - PyObject* joiner; - PyObject* function; - PyObject* args; PyObject* result; - switch (PyList_GET_SIZE(list)) { - case 0: - Py_DECREF(list); - return PyBytes_FromString(""); - case 1: - result = PyList_GET_ITEM(list, 0); - Py_INCREF(result); - Py_DECREF(list); - return result; - } - - /* two or more elements: slice out a suitable separator from the - first member, and use that to join the entire list */ - - joiner = PySequence_GetSlice(PyList_GET_ITEM(list, 0), 0, 0); + joiner = PyUnicode_FromStringAndSize("", 0); if (!joiner) return NULL; - - function = PyObject_GetAttrString(joiner, "join"); - if (!function) { - Py_DECREF(joiner); - return NULL; - } - - args = PyTuple_New(1); - if (!args) - return NULL; - - PyTuple_SET_ITEM(args, 0, list); - - result = PyObject_CallObject(function, args); - - Py_DECREF(args); /* also removes list */ - Py_DECREF(function); + result = PyUnicode_Join(joiner, list); Py_DECREF(joiner); - + if (result) + Py_DECREF(list); return result; } @@ -399,6 +361,7 @@ element_init(PyObject *self, PyObject *args, PyObject *kwds) return -1; if (kwds) { if (PyDict_Update(attrib, kwds) < 0) { + Py_DECREF(attrib); return -1; } } @@ -407,38 +370,34 @@ element_init(PyObject *self, PyObject *args, PyObject *kwds) attrib = get_attrib_from_keywords(kwds); if (!attrib) return -1; - } else { - /* no attrib arg, no kwds, so no attributes */ - Py_INCREF(Py_None); - attrib = Py_None; } self_elem = (ElementObject *)self; - if (attrib != Py_None && !is_empty_dict(attrib)) { + if (attrib != NULL && !is_empty_dict(attrib)) { if (create_extra(self_elem, attrib) < 0) { - PyObject_Del(self_elem); + Py_DECREF(attrib); return -1; } } /* We own a reference to attrib here and it's no longer needed. */ - Py_DECREF(attrib); + Py_XDECREF(attrib); /* Replace the objects already pointed to by tag, text and tail. */ tmp = self_elem->tag; - self_elem->tag = tag; Py_INCREF(tag); + self_elem->tag = tag; Py_DECREF(tmp); tmp = self_elem->text; - self_elem->text = Py_None; Py_INCREF(Py_None); + self_elem->text = Py_None; Py_DECREF(JOIN_OBJ(tmp)); tmp = self_elem->tail; - self_elem->tail = Py_None; Py_INCREF(Py_None); + self_elem->tail = Py_None; Py_DECREF(JOIN_OBJ(tmp)); return 0; @@ -520,11 +479,11 @@ element_get_attrib(ElementObject* self) PyObject* res = self->extra->attrib; if (res == Py_None) { - Py_DECREF(res); /* create missing dictionary */ res = PyDict_New(); if (!res) return NULL; + Py_DECREF(Py_None); self->extra->attrib = res; } @@ -824,7 +783,7 @@ element_deepcopy(ElementObject* self, PyObject* args) } /* add object to memo dictionary (so deepcopy won't visit it again) */ - id = PyLong_FromLong((Py_uintptr_t) self); + id = PyLong_FromSsize_t((Py_uintptr_t) self); if (!id) goto error; @@ -2081,6 +2040,7 @@ treebuilder_new(PyTypeObject *type, PyObject *args, PyObject *kwds) if (!t->stack) { Py_DECREF(t->this); Py_DECREF(t->last); + Py_DECREF((PyObject *) t); return NULL; } t->index = 0; @@ -2098,6 +2058,7 @@ treebuilder_init(PyObject *self, PyObject *args, PyObject *kwds) static char *kwlist[] = {"element_factory", 0}; PyObject *element_factory = NULL; TreeBuilderObject *self_tb = (TreeBuilderObject *)self; + PyObject *tmp; if (!PyArg_ParseTupleAndKeywords(args, kwds, "|O:TreeBuilder", kwlist, &element_factory)) { @@ -2106,8 +2067,9 @@ treebuilder_init(PyObject *self, PyObject *args, PyObject *kwds) if (element_factory) { Py_INCREF(element_factory); - Py_XDECREF(self_tb->element_factory); + tmp = self_tb->element_factory; self_tb->element_factory = element_factory; + Py_XDECREF(tmp); } return 0; @@ -2128,17 +2090,17 @@ treebuilder_gc_traverse(TreeBuilderObject *self, visitproc visit, void *arg) static int treebuilder_gc_clear(TreeBuilderObject *self) { - Py_XDECREF(self->end_ns_event_obj); - Py_XDECREF(self->start_ns_event_obj); - Py_XDECREF(self->end_event_obj); - Py_XDECREF(self->start_event_obj); - Py_XDECREF(self->events); - Py_DECREF(self->stack); - Py_XDECREF(self->data); - Py_DECREF(self->last); - Py_DECREF(self->this); + Py_CLEAR(self->end_ns_event_obj); + Py_CLEAR(self->start_ns_event_obj); + Py_CLEAR(self->end_event_obj); + Py_CLEAR(self->start_event_obj); + Py_CLEAR(self->events); + Py_CLEAR(self->stack); + Py_CLEAR(self->data); + Py_CLEAR(self->last); + Py_CLEAR(self->this); Py_CLEAR(self->element_factory); - Py_XDECREF(self->root); + Py_CLEAR(self->root); return 0; } @@ -2222,10 +2184,8 @@ treebuilder_handle_start(TreeBuilderObject* self, PyObject* tag, if (self->start_event_obj) { PyObject* res; PyObject* action = self->start_event_obj; - res = PyTuple_New(2); + res = PyTuple_Pack(2, action, node); if (res) { - Py_INCREF(action); PyTuple_SET_ITEM(res, 0, (PyObject*) action); - Py_INCREF(node); PyTuple_SET_ITEM(res, 1, (PyObject*) node); PyList_Append(self->events, res); Py_DECREF(res); } else @@ -2253,6 +2213,7 @@ treebuilder_handle_data(TreeBuilderObject* self, PyObject* data) /* more than one item; use a list to collect items */ if (PyBytes_CheckExact(self->data) && Py_REFCNT(self->data) == 1 && PyBytes_CheckExact(data) && PyBytes_GET_SIZE(data) == 1) { + /* XXX this code path unused in Python 3? */ /* expat often generates single character data sections; handle the most common case by resizing the existing string... */ Py_ssize_t size = PyBytes_GET_SIZE(self->data); @@ -2317,10 +2278,8 @@ treebuilder_handle_end(TreeBuilderObject* self, PyObject* tag) PyObject* res; PyObject* action = self->end_event_obj; PyObject* node = (PyObject*) self->last; - res = PyTuple_New(2); + res = PyTuple_Pack(2, action, node); if (res) { - Py_INCREF(action); PyTuple_SET_ITEM(res, 0, (PyObject*) action); - Py_INCREF(node); PyTuple_SET_ITEM(res, 1, (PyObject*) node); PyList_Append(self->events, res); Py_DECREF(res); } else @@ -2366,8 +2325,12 @@ treebuilder_handle_namespace(TreeBuilderObject* self, int start, PyTuple_SET_ITEM(res, 1, parcel); PyList_Append(self->events, res); Py_DECREF(res); - } else + } + else { + Py_DECREF(action); + Py_DECREF(parcel); PyErr_Clear(); /* FIXME: propagate error */ + } } /* -------------------------------------------------------------------- */ @@ -2526,7 +2489,7 @@ makeuniversal(XMLParserObject* self, const char* string) /* convert a UTF-8 tag/attribute name from the expat parser to a universal name string */ - int size = strlen(string); + Py_ssize_t size = (Py_ssize_t) strlen(string); PyObject* key; PyObject* value; @@ -2545,7 +2508,7 @@ makeuniversal(XMLParserObject* self, const char* string) PyObject* tag; char* p; - int i; + Py_ssize_t i; /* look for namespace separator */ for (i = 0; i < size; i++) @@ -2717,13 +2680,7 @@ expat_start_handler(XMLParserObject* self, const XML_Char* tag_in, attrib_in += 2; } } else { - Py_INCREF(Py_None); - attrib = Py_None; - } - - /* If we get None, pass an empty dictionary on */ - if (attrib == Py_None) { - Py_DECREF(attrib); + /* Pass an empty dictionary on */ attrib = PyDict_New(); if (!attrib) return; @@ -3015,14 +2972,14 @@ xmlparser_init(PyObject *self, PyObject *args, PyObject *kwds) self_xp->names = PyDict_New(); if (!self_xp->names) { - Py_XDECREF(self_xp->entity); + Py_CLEAR(self_xp->entity); return -1; } self_xp->parser = EXPAT(ParserCreate_MM)(encoding, &ExpatMemoryHandler, "}"); if (!self_xp->parser) { - Py_XDECREF(self_xp->entity); - Py_XDECREF(self_xp->names); + Py_CLEAR(self_xp->entity); + Py_CLEAR(self_xp->names); PyErr_NoMemory(); return -1; } @@ -3032,8 +2989,8 @@ xmlparser_init(PyObject *self, PyObject *args, PyObject *kwds) } else { target = treebuilder_new(&TreeBuilder_Type, NULL, NULL); if (!target) { - Py_XDECREF(self_xp->entity); - Py_XDECREF(self_xp->names); + Py_CLEAR(self_xp->entity); + Py_CLEAR(self_xp->names); EXPAT(ParserFree)(self_xp->parser); return -1; } @@ -3109,17 +3066,17 @@ xmlparser_gc_clear(XMLParserObject *self) { EXPAT(ParserFree)(self->parser); - Py_XDECREF(self->handle_close); - Py_XDECREF(self->handle_pi); - Py_XDECREF(self->handle_comment); - Py_XDECREF(self->handle_end); - Py_XDECREF(self->handle_data); - Py_XDECREF(self->handle_start); - Py_XDECREF(self->handle_doctype); + Py_CLEAR(self->handle_close); + Py_CLEAR(self->handle_pi); + Py_CLEAR(self->handle_comment); + Py_CLEAR(self->handle_end); + Py_CLEAR(self->handle_data); + Py_CLEAR(self->handle_start); + Py_CLEAR(self->handle_doctype); - Py_XDECREF(self->target); - Py_XDECREF(self->entity); - Py_XDECREF(self->names); + Py_CLEAR(self->target); + Py_CLEAR(self->entity); + Py_CLEAR(self->names); return 0; } @@ -3227,17 +3184,12 @@ xmlparser_parse(XMLParserObject* self, PyObject* args) break; } temp = PyUnicode_AsEncodedString(buffer, "utf-8", "surrogatepass"); + Py_DECREF(buffer); if (!temp) { /* Propagate exception from PyUnicode_AsEncodedString */ - Py_DECREF(buffer); Py_DECREF(reader); return NULL; } - - /* Here we no longer need the original buffer since it contains - * unicode. Make it point to the encoded bytes object. - */ - Py_DECREF(buffer); buffer = temp; } else if (!PyBytes_CheckExact(buffer) || PyBytes_GET_SIZE(buffer) == 0) { @@ -3307,10 +3259,10 @@ xmlparser_setevents(XMLParserObject *self, PyObject* args) target->events = events; /* clear out existing events */ - Py_XDECREF(target->start_event_obj); target->start_event_obj = NULL; - Py_XDECREF(target->end_event_obj); target->end_event_obj = NULL; - Py_XDECREF(target->start_ns_event_obj); target->start_ns_event_obj = NULL; - Py_XDECREF(target->end_ns_event_obj); target->end_ns_event_obj = NULL; + Py_CLEAR(target->start_event_obj); + Py_CLEAR(target->end_event_obj); + Py_CLEAR(target->start_ns_event_obj); + Py_CLEAR(target->end_ns_event_obj); if (event_set == Py_None) { /* default is "end" only */