Addressed PR comments.

pull/13171/head
Joshua Haberman 3 years ago
parent 83d1508ee5
commit d6212461e6
  1. 58
      python/descriptor_containers.c

@ -159,18 +159,19 @@ static int PyUpb_GenericSequence_IsEqual(PyUpb_GenericSequence *self,
if (!PyList_Check(other)) return 0; if (!PyList_Check(other)) return 0;
// return list(self) == other // return list(self) == other
// We can clamp `i` to int because GenericSequence uses int for size (this
// is useful when we do int iteration below).
int n = PyUpb_GenericSequence_Length((PyObject*)self); int n = PyUpb_GenericSequence_Length((PyObject*)self);
if (n != PyList_Size(other)) { if ((Py_ssize_t)n != PyList_Size(other)) {
return false; return false;
} }
PyObject *item1;
PyObject *item2;
for (int i = 0; i < n; i++) { for (int i = 0; i < n; i++) {
PyObject *item1 = PyUpb_GenericSequence_GetItem((PyObject*)self, i); item1 = PyUpb_GenericSequence_GetItem((PyObject*)self, i);
if (!item1) return -1; item2 = PyList_GetItem(other, i);
PyObject* item2 = PyList_GetItem(other, i); if (!item1 || !item2) goto error;
if (!item2) {
Py_DECREF(item1);
return -1;
}
int cmp = PyObject_RichCompareBool(item1, item2, Py_EQ); int cmp = PyObject_RichCompareBool(item1, item2, Py_EQ);
Py_DECREF(item1); Py_DECREF(item1);
Py_DECREF(item2); Py_DECREF(item2);
@ -178,6 +179,11 @@ static int PyUpb_GenericSequence_IsEqual(PyUpb_GenericSequence *self,
} }
// All items were found and equal // All items were found and equal
return 1; return 1;
error:
if (item1) Py_DECREF(item1);
if (item2) Py_DECREF(item2);
return -1;
} }
static PyObject *PyUpb_GenericSequence_RichCompare(PyObject *_self, static PyObject *PyUpb_GenericSequence_RichCompare(PyObject *_self,
@ -215,8 +221,6 @@ static PyObject* PyUpb_GenericSequence_Index(PyObject* self, PyObject* item) {
} }
} }
// Implements list.count(): number of occurrences of the item in the sequence.
// An item can only appear once in a sequence. If it exists, return 1.
static PyObject *PyUpb_GenericSequence_Count(PyObject *_self, PyObject *item) { static PyObject *PyUpb_GenericSequence_Count(PyObject *_self, PyObject *item) {
PyUpb_GenericSequence *self = PyUpb_GenericSequence_Self(_self); PyUpb_GenericSequence *self = PyUpb_GenericSequence_Self(_self);
const void *item_ptr = PyUpb_AnyDescriptor_GetDef(item); const void *item_ptr = PyUpb_AnyDescriptor_GetDef(item);
@ -377,7 +381,7 @@ static PyObject *PyUpb_ByNameMap_Values(PyObject *_self, PyObject *args) {
for (int i = 0; i < n; i++) { for (int i = 0; i < n; i++) {
const void *elem = self->funcs->base.index(self->parent, i); const void *elem = self->funcs->base.index(self->parent, i);
PyObject *py_elem = self->funcs->base.get_elem_wrapper(elem); PyObject *py_elem = self->funcs->base.get_elem_wrapper(elem);
if (!elem) { if (!py_elem) {
Py_DECREF(ret); Py_DECREF(ret);
return NULL; return NULL;
} }
@ -391,13 +395,13 @@ static PyObject *PyUpb_ByNameMap_Items(PyObject *_self, PyObject *args) {
int n = self->funcs->base.get_elem_count(self->parent); int n = self->funcs->base.get_elem_count(self->parent);
PyObject *ret = PyList_New(n); PyObject *ret = PyList_New(n);
PyObject *item; PyObject *item;
PyObject *py_elem;
if (!ret) return NULL; if (!ret) return NULL;
for (int i = 0; i < n; i++) { for (int i = 0; i < n; i++) {
item = PyTuple_New(2);
if (!item) goto err1;
const void *elem = self->funcs->base.index(self->parent, i); const void *elem = self->funcs->base.index(self->parent, i);
PyObject *py_elem = self->funcs->base.get_elem_wrapper(elem); item = PyTuple_New(2);
if (!py_elem) goto err2; py_elem = self->funcs->base.get_elem_wrapper(elem);
if (!item || !py_elem) goto error;
PyTuple_SetItem(item, 0, PyTuple_SetItem(item, 0,
PyUnicode_FromString(self->funcs->get_elem_name(elem))); PyUnicode_FromString(self->funcs->get_elem_name(elem)));
PyTuple_SetItem(item, 1, py_elem); PyTuple_SetItem(item, 1, py_elem);
@ -405,10 +409,10 @@ static PyObject *PyUpb_ByNameMap_Items(PyObject *_self, PyObject *args) {
} }
return ret; return ret;
err2: error:
Py_DECREF(item); if (!py_elem) Py_DECREF(py_elem);
err1: if (item) Py_DECREF(item);
Py_DECREF(ret); if (ret) Py_DECREF(ret);
return NULL; return NULL;
} }
@ -597,24 +601,24 @@ static PyObject *PyUpb_ByNumberMap_Items(PyObject *_self, PyObject *args) {
int n = self->funcs->base.get_elem_count(self->parent); int n = self->funcs->base.get_elem_count(self->parent);
PyObject *ret = PyList_New(n); PyObject *ret = PyList_New(n);
PyObject *item; PyObject *item;
PyObject *py_elem;
if (!ret) return NULL; if (!ret) return NULL;
for (int i = 0; i < n; i++) { for (int i = 0; i < n; i++) {
item = PyTuple_New(2);
if (!item) goto err1;
const void *elem = self->funcs->base.index(self->parent, i); const void *elem = self->funcs->base.index(self->parent, i);
int number = self->funcs->get_elem_num(elem); int number = self->funcs->get_elem_num(elem);
PyObject *py_elem = self->funcs->base.get_elem_wrapper(elem); item = PyTuple_New(2);
if (!py_elem) goto err2; py_elem = self->funcs->base.get_elem_wrapper(elem);
if (!item || ! py_elem) goto error;
PyTuple_SetItem(item, 0, PyLong_FromLong(number)); PyTuple_SetItem(item, 0, PyLong_FromLong(number));
PyTuple_SetItem(item, 1, py_elem); PyTuple_SetItem(item, 1, py_elem);
PyList_SetItem(ret, i, item); PyList_SetItem(ret, i, item);
} }
return ret; return ret;
err2: error:
Py_DECREF(item); if (py_elem) Py_DECREF(py_elem);
err1: if (item) Py_DECREF(item);
Py_DECREF(ret); if (ret) Py_DECREF(ret);
return NULL; return NULL;
} }

Loading…
Cancel
Save