bpo-39822: Use NULL instead of None for empty attrib in Element. (GH-18735)

Author: serhiy-storchaka

Modules/_elementtree.c

@@ -170,7 +170,7 @@ is_empty_dict(PyObject *obj)
 
 typedef struct {
 
-    /* attributes (a dictionary object), or None if no attributes */
+    /* attributes (a dictionary object), or NULL if no attributes */
     PyObject* attrib;
 
     /* child elements */
@@ -225,10 +225,7 @@ create_extra(ElementObject* self, PyObject* attrib)
         return -1;
     }
 
-    if (!attrib)
-        attrib = Py_None;
-
-    Py_INCREF(attrib);
+    Py_XINCREF(attrib);
     self->extra->attrib = attrib;
 
     self->extra->length = 0;
@@ -246,7 +243,7 @@ dealloc_extra(ElementObjectExtra *extra)
     if (!extra)
         return;
 
-    Py_DECREF(extra->attrib);
+    Py_XDECREF(extra->attrib);
 
     for (i = 0; i < extra->length; i++)
         Py_DECREF(extra->children[i]);
@@ -300,7 +297,7 @@ create_new_element(PyObject* tag, PyObject* attrib)
     ALLOC(sizeof(ElementObject), "create element");
     PyObject_GC_Track(self);
 
-    if (attrib != Py_None && !is_empty_dict(attrib)) {
+    if (attrib != NULL && !is_empty_dict(attrib)) {
         if (create_extra(self, attrib) < 0) {
             Py_DECREF(self);
             return NULL;
@@ -530,13 +527,9 @@ element_get_attrib(ElementObject* self)
 
     PyObject* res = self->extra->attrib;
 
-    if (res == Py_None) {
+    if (!res) {
         /* create missing dictionary */
-        res = PyDict_New();
-        if (!res)
-            return NULL;
-        Py_DECREF(Py_None);
-        self->extra->attrib = res;
+        res = self->extra->attrib = PyDict_New();
     }
 
     return res;
@@ -616,12 +609,10 @@ subelement(PyObject *self, PyObject *args, PyObject *kwds)
             return NULL;
     } else {
         /* no attrib arg, no kwds, so no attribute */
-        Py_INCREF(Py_None);
-        attrib = Py_None;
     }
 
     elem = create_new_element(tag, attrib);
-    Py_DECREF(attrib);
+    Py_XDECREF(attrib);
     if (elem == NULL)
         return NULL;
 
@@ -736,7 +727,7 @@ _elementtree_Element___copy___impl(ElementObject *self)
     ElementObject* element;
 
     element = (ElementObject*) create_new_element(
-        self->tag, (self->extra) ? self->extra->attrib : Py_None);
+        self->tag, self->extra ? self->extra->attrib : NULL);
     if (!element)
         return NULL;
 
@@ -792,21 +783,20 @@ _elementtree_Element___deepcopy___impl(ElementObject *self, PyObject *memo)
     if (!tag)
         return NULL;
 
-    if (self->extra) {
+    if (self->extra && self->extra->attrib) {
         attrib = deepcopy(self->extra->attrib, memo);
         if (!attrib) {
             Py_DECREF(tag);
             return NULL;
         }
     } else {
-        Py_INCREF(Py_None);
-        attrib = Py_None;
+        attrib = NULL;
     }
 
     element = (ElementObject*) create_new_element(tag, attrib);
 
     Py_DECREF(tag);
-    Py_DECREF(attrib);
+    Py_XDECREF(attrib);
 
     if (!element)
         return NULL;
@@ -963,7 +953,7 @@ _elementtree_Element___getstate___impl(ElementObject *self)
         PyList_SET_ITEM(children, i, child);
     }
 
-    if (self->extra && self->extra->attrib != Py_None) {
+    if (self->extra && self->extra->attrib) {
         attrib = self->extra->attrib;
         Py_INCREF(attrib);
     }
@@ -1037,9 +1027,9 @@ element_setstate_from_attributes(ElementObject *self,
         assert(self->extra);
         assert(self->extra->allocated >= nchildren);
         if (oldextra) {
-            assert(self->extra->attrib == Py_None);
+            assert(self->extra->attrib == NULL);
             self->extra->attrib = oldextra->attrib;
-            oldextra->attrib = Py_None;
+            oldextra->attrib = NULL;
         }
 
         /* Copy children */
@@ -1065,10 +1055,8 @@ element_setstate_from_attributes(ElementObject *self,
     }
 
     /* Stash attrib. */
-    if (attrib) {
-        Py_INCREF(attrib);
-        Py_XSETREF(self->extra->attrib, attrib);
-    }
+    Py_XINCREF(attrib);
+    Py_XSETREF(self->extra->attrib, attrib);
     dealloc_extra(oldextra);
 
     Py_RETURN_NONE;
@@ -1401,7 +1389,7 @@ _elementtree_Element_get_impl(ElementObject *self, PyObject *key,
 {
     PyObject* value;
 
-    if (!self->extra || self->extra->attrib == Py_None)
+    if (!self->extra || !self->extra->attrib)
         value = default_value;
     else {
         value = PyDict_GetItemWithError(self->extra->attrib, key);
@@ -1529,7 +1517,7 @@ static PyObject *
 _elementtree_Element_items_impl(ElementObject *self)
 /*[clinic end generated code: output=6db2c778ce3f5a4d input=adbe09aaea474447]*/
 {
-    if (!self->extra || self->extra->attrib == Py_None)
+    if (!self->extra || !self->extra->attrib)
         return PyList_New(0);
 
     return PyDict_Items(self->extra->attrib);
@@ -1544,7 +1532,7 @@ static PyObject *
 _elementtree_Element_keys_impl(ElementObject *self)
 /*[clinic end generated code: output=bc5bfabbf20eeb3c input=f02caf5b496b5b0b]*/
 {
-    if (!self->extra || self->extra->attrib == Py_None)
+    if (!self->extra || !self->extra->attrib)
         return PyList_New(0);
 
     return PyDict_Keys(self->extra->attrib);
@@ -1563,15 +1551,15 @@ element_length(ElementObject* self)
 _elementtree.Element.makeelement
 
     tag: object
-    attrib: object
+    attrib: object(subclass_of='&PyDict_Type')
     /
 
 [clinic start generated code]*/
 
 static PyObject *
 _elementtree_Element_makeelement_impl(ElementObject *self, PyObject *tag,
                                       PyObject *attrib)
-/*[clinic end generated code: output=4109832d5bb789ef input=9480d1d2e3e68235]*/
+/*[clinic end generated code: output=4109832d5bb789ef input=2279d974529c3861]*/
 {
     PyObject* elem;
 
@@ -2043,12 +2031,18 @@ static int
 element_attrib_setter(ElementObject *self, PyObject *value, void *closure)
 {
     _VALIDATE_ATTR_VALUE(value);
+    if (!PyDict_Check(value)) {
+        PyErr_Format(PyExc_TypeError,
+                     "attrib must be dict, not %.200s",
+                     value->ob_type->tp_name);
+        return -1;
+    }
     if (!self->extra) {
         if (create_extra(self, NULL) < 0)
             return -1;
     }
     Py_INCREF(value);
-    Py_SETREF(self->extra->attrib, value);
+    Py_XSETREF(self->extra->attrib, value);
     return 0;
 }
 
@@ -2688,7 +2682,7 @@ treebuilder_handle_start(TreeBuilderObject* self, PyObject* tag,
 
     if (!self->element_factory) {
         node = create_new_element(tag, attrib);
-    } else if (attrib == Py_None) {
+    } else if (attrib == NULL) {
         attrib = PyDict_New();
         if (!attrib)
             return NULL;
@@ -3297,8 +3291,7 @@ expat_start_handler(XMLParserObject* self, const XML_Char* tag_in,
             attrib_in += 2;
         }
     } else {
-        Py_INCREF(Py_None);
-        attrib = Py_None;
+        attrib = NULL;
     }
 
     if (TreeBuilder_CheckExact(self->target)) {
@@ -3307,8 +3300,7 @@ expat_start_handler(XMLParserObject* self, const XML_Char* tag_in,
                                        tag, attrib);
     }
     else if (self->handle_start) {
-        if (attrib == Py_None) {
-            Py_DECREF(attrib);
+        if (attrib == NULL) {
             attrib = PyDict_New();
             if (!attrib) {
                 Py_DECREF(tag);
@@ -3321,7 +3313,7 @@ expat_start_handler(XMLParserObject* self, const XML_Char* tag_in,
         res = NULL;
 
     Py_DECREF(tag);
-    Py_DECREF(attrib);
+    Py_XDECREF(attrib);
 
     Py_XDECREF(res);
 }