From c264f9fa3742e5c05e4552d34a150a14ae0d2b11 Mon Sep 17 00:00:00 2001 From: Ilan Schnell Date: Thu, 9 Nov 2023 15:10:15 -0600 Subject: [PATCH 1/5] ensure readonly attribute is handled correctly when creating new objects --- bitarray/_bitarray.c | 122 +++++++++++++++++++++----------------- bitarray/test_bitarray.py | 9 +++ 2 files changed, 78 insertions(+), 53 deletions(-) diff --git a/bitarray/_bitarray.c b/bitarray/_bitarray.c index 80fa22d3..02beffb6 100644 --- a/bitarray/_bitarray.c +++ b/bitarray/_bitarray.c @@ -110,14 +110,16 @@ resize(bitarrayobject *self, Py_ssize_t nbits) } /* create new bitarray object without initialization of buffer */ -static PyObject * +static bitarrayobject * newbitarrayobject(PyTypeObject *type, Py_ssize_t nbits, int endian) { const Py_ssize_t nbytes = BYTES(nbits); bitarrayobject *obj; - if (nbits < 0 || nbytes < 0) - return PyErr_Format(PyExc_OverflowError, "new bitarray %zd", nbits); + if (nbits < 0 || nbytes < 0) { + PyErr_Format(PyExc_OverflowError, "new bitarray %zd", nbits); + return NULL; + } obj = (bitarrayobject *) type->tp_alloc(type, 0); if (obj == NULL) @@ -131,7 +133,8 @@ newbitarrayobject(PyTypeObject *type, Py_ssize_t nbits, int endian) obj->ob_item = (char *) PyMem_Malloc((size_t) nbytes); if (obj->ob_item == NULL) { PyObject_Del(obj); - return PyErr_NoMemory(); + PyErr_NoMemory(); + return NULL; } } obj->allocated = nbytes; @@ -141,7 +144,7 @@ newbitarrayobject(PyTypeObject *type, Py_ssize_t nbits, int endian) obj->weakreflist = NULL; obj->buffer = NULL; obj->readonly = 0; - return (PyObject *) obj; + return obj; } static void @@ -944,15 +947,15 @@ Remove all items from the bitarray."); static PyObject * bitarray_copy(bitarrayobject *self) { - PyObject *res; + bitarrayobject *res; res = newbitarrayobject(Py_TYPE(self), self->nbits, self->endian); if (res == NULL) return NULL; - memcpy(((bitarrayobject *) res)->ob_item, self->ob_item, - (size_t) Py_SIZE(self)); - return res; + memcpy(res->ob_item, self->ob_item, (size_t) Py_SIZE(self)); + res->readonly = self->readonly; + return (PyObject *) res; } PyDoc_STRVAR(copy_doc, @@ -1283,8 +1286,7 @@ searcharg(PyObject *x) if (!conv_pybit(x, &vi)) return NULL; - xa = (bitarrayobject *) newbitarrayobject(&Bitarray_Type, 1, - ENDIAN_LITTLE); + xa = newbitarrayobject(&Bitarray_Type, 1, ENDIAN_LITTLE); if (xa == NULL) return NULL; setbit(xa, 0, vi); @@ -1829,33 +1831,41 @@ bitarray_len(bitarrayobject *self) static PyObject * bitarray_concat(bitarrayobject *self, PyObject *other) { - PyObject *res; + bitarrayobject *res; - res = bitarray_copy(self); + res = (bitarrayobject *) bitarray_copy(self); if (res == NULL) return NULL; - if (extend_dispatch((bitarrayobject *) res, other) < 0) { + res->readonly = 0; + if (extend_dispatch(res, other) < 0) { Py_DECREF(res); return NULL; } - return res; + set_padbits(res); + res->readonly = self->readonly; + + return (PyObject *) res; } static PyObject * bitarray_repeat(bitarrayobject *self, Py_ssize_t n) { - PyObject *res; + bitarrayobject *res; - res = bitarray_copy(self); + res = (bitarrayobject *) bitarray_copy(self); if (res == NULL) return NULL; - if (repeat((bitarrayobject *) res, n) < 0) { + res->readonly = 0; + if (repeat(res, n) < 0) { Py_DECREF(res); return NULL; } - return res; + set_padbits(res); + res->readonly = self->readonly; + + return (PyObject *) res; } static PyObject * @@ -1937,7 +1947,7 @@ static PyObject * getslice(bitarrayobject *self, PyObject *slice) { Py_ssize_t start, stop, step, slicelength; - PyObject *res; + bitarrayobject *res; assert(PySlice_Check(slice)); if (PySlice_GetIndicesEx(slice, self->nbits, @@ -1948,18 +1958,19 @@ getslice(bitarrayobject *self, PyObject *slice) if (res == NULL) return NULL; -#define rr ((bitarrayobject *) res) if (step == 1) { - copy_n(rr, 0, self, start, slicelength); + copy_n(res, 0, self, start, slicelength); } else { Py_ssize_t i, j; for (i = 0, j = start; i < slicelength; i++, j += step) - setbit(rr, i, getbit(self, j)); + setbit(res, i, getbit(self, j)); } -#undef rr - return res; + set_padbits(res); + res->readonly = self->readonly; + + return (PyObject *) res; } static int @@ -1977,7 +1988,7 @@ check_mask_length(bitarrayobject *self, bitarrayobject *mask) static PyObject * getmasked(bitarrayobject *self, bitarrayobject *mask) { - PyObject *res; + bitarrayobject *res; Py_ssize_t i, j, n; if (check_mask_length(self, mask) < 0) @@ -1990,10 +2001,12 @@ getmasked(bitarrayobject *self, bitarrayobject *mask) for (i = j = 0; i < mask->nbits; i++) { if (getbit(mask, i)) - setbit((bitarrayobject *) res, j++, getbit(self, i)); + setbit(res, j++, getbit(self, i)); } + set_padbits(res); + res->readonly = self->readonly; assert(j == n); - return res; + return (PyObject *) res; } /* Return j-th item from sequence. The item is considered an index into @@ -2026,7 +2039,7 @@ index_from_seq(PyObject *sequence, Py_ssize_t j, Py_ssize_t length) static PyObject * getsequence(bitarrayobject *self, PyObject *seq) { - PyObject *res; + bitarrayobject *res; Py_ssize_t i, j, n; n = PySequence_Size(seq); @@ -2039,9 +2052,12 @@ getsequence(bitarrayobject *self, PyObject *seq) Py_DECREF(res); return NULL; } - setbit((bitarrayobject *) res, j, getbit(self, i)); + setbit(res, j, getbit(self, i)); } - return res; + set_padbits(res); + res->readonly = self->readonly; + + return (PyObject *) res; } static int @@ -2337,8 +2353,7 @@ delsequence(bitarrayobject *self, PyObject *seq) return 0; /* create mask bitarray - note that it's endianness is irrelevant */ - mask = (bitarrayobject *) newbitarrayobject(&Bitarray_Type, self->nbits, - ENDIAN_LITTLE); + mask = newbitarrayobject(&Bitarray_Type, self->nbits, ENDIAN_LITTLE); if (mask == NULL) return -1; memset(mask->ob_item, 0x00, (size_t) Py_SIZE(mask)); @@ -2981,7 +2996,8 @@ decodetree_new(PyTypeObject *type, PyObject *args, PyObject *kwds) static PyObject * decodetree_todict(decodetreeobject *self) { - PyObject *dict, *prefix; + PyObject *dict; + bitarrayobject *prefix; dict = PyDict_New(); if (dict == NULL) @@ -2991,7 +3007,7 @@ decodetree_todict(decodetreeobject *self) if (prefix == NULL) goto error; - if (binode_to_dict(self->tree, dict, (bitarrayobject *) prefix) < 0) + if (binode_to_dict(self->tree, dict, prefix) < 0) goto error; Py_DECREF(prefix); @@ -3570,7 +3586,7 @@ newbitarray_from_index(PyTypeObject *type, PyObject *index, int endian) return NULL; } - return newbitarrayobject(type, nbits, endian); + return (PyObject *) newbitarrayobject(type, nbits, endian); } /* Return a new bitarray from pickle bytes (created by .__reduce__()). @@ -3585,7 +3601,7 @@ newbitarray_from_index(PyTypeObject *type, PyObject *index, int endian) static PyObject * newbitarray_from_pickle(PyTypeObject *type, PyObject *bytes, char *endian_str) { - PyObject *res; + bitarrayobject *res; Py_ssize_t nbytes; char *data; unsigned char head; @@ -3615,14 +3631,15 @@ newbitarray_from_pickle(PyTypeObject *type, PyObject *bytes, char *endian_str) endian); if (res == NULL) return NULL; - memcpy(((bitarrayobject *) res)->ob_item, data + 1, (size_t) nbytes - 1); - return res; + memcpy(res->ob_item, data + 1, (size_t) nbytes - 1); + return (PyObject *) res; } static PyObject * bitarray_new(PyTypeObject *type, PyObject *args, PyObject *kwds) { - PyObject *result, *initial = Py_None, *buffer = Py_None; + PyObject *initial = Py_None, *buffer = Py_None; + bitarrayobject *res; char *endian_str = NULL; int endian; static char *kwlist[] = {"", "endian", "buffer", NULL}; @@ -3646,7 +3663,7 @@ bitarray_new(PyTypeObject *type, PyObject *args, PyObject *kwds) /* no arg / None */ if (initial == Py_None) - return newbitarrayobject(type, 0, endian); + return (PyObject *) newbitarrayobject(type, 0, endian); /* bool */ if (PyBool_Check(initial)) { @@ -3670,14 +3687,14 @@ bitarray_new(PyTypeObject *type, PyObject *args, PyObject *kwds) endian = ((bitarrayobject *) initial)->endian; /* leave remaining type dispatch to extend method */ - result = newbitarrayobject(type, 0, endian); - if (result == NULL) + res = newbitarrayobject(type, 0, endian); + if (res == NULL) return NULL; - if (extend_dispatch((bitarrayobject *) result, initial) < 0) { - Py_DECREF(result); + if (extend_dispatch(res, initial) < 0) { + Py_DECREF(res); return NULL; } - return result; + return (PyObject *) res; } static int @@ -4026,7 +4043,8 @@ reconstructor(PyObject *module, PyObject *args) { PyTypeObject *type; Py_ssize_t nbytes; - PyObject *res, *bytes; + PyObject *bytes; + bitarrayobject *res; char *endian_str; int endian, padbits, readonly; @@ -4057,14 +4075,12 @@ reconstructor(PyObject *module, PyObject *args) res = newbitarrayobject(type, 8 * nbytes - padbits, endian); if (res == NULL) return NULL; -#define rr ((bitarrayobject *) res) - memcpy(rr->ob_item, PyBytes_AS_STRING(bytes), (size_t) nbytes); + memcpy(res->ob_item, PyBytes_AS_STRING(bytes), (size_t) nbytes); if (readonly) { - set_padbits(rr); - rr->readonly = 1; + set_padbits(res); + res->readonly = 1; } -#undef rr - return res; + return (PyObject *) res; } diff --git a/bitarray/test_bitarray.py b/bitarray/test_bitarray.py index 24924df0..9ec91816 100644 --- a/bitarray/test_bitarray.py +++ b/bitarray/test_bitarray.py @@ -4875,6 +4875,15 @@ def test_immutable(self): self.assertRaises(TypeError, a.__ilshift__, 1) self.check_obj(a) + def test_copy(self): + a = frozenbitarray('101') + # not only .copy() creates new frozenbitarray which are read-only + for b in [a, a.copy(), 3 * a, 5 * a, a & bitarray('110'), + a + bitarray(8*'1'), a[:], a[[0, 1]], a[bitarray('011')]]: + self.assertIsType(b, 'frozenbitarray') + self.assertTrue(b.readonly) + self.check_obj(b) + def test_freeze(self): # not so much a test for frozenbitarray, but how it is initialized a = bitarray(78) From d82956fbfa3f029da63390d9f6954061983a7d4d Mon Sep 17 00:00:00 2001 From: Ilan Schnell Date: Thu, 9 Nov 2023 22:57:02 -0600 Subject: [PATCH 2/5] add set_readonly() --- bitarray/_bitarray.c | 48 ++++++++++++++++++++++++++++++++------- bitarray/test_bitarray.py | 7 ++++++ 2 files changed, 47 insertions(+), 8 deletions(-) diff --git a/bitarray/_bitarray.c b/bitarray/_bitarray.c index 02beffb6..d1f4ea62 100644 --- a/bitarray/_bitarray.c +++ b/bitarray/_bitarray.c @@ -784,6 +784,31 @@ extend_dispatch(bitarrayobject *self, PyObject *obj) return -1; } +/* set the readonly member to 0 or 1 depending on whether self in an instance + of the frozenbitarray class - on error, return -1 and set an exception. */ +static int +set_readonly(bitarrayobject *self) +{ + static PyObject *frozen = NULL; /* frozenbitarray class object */ + int is_frozen; + + if (frozen == NULL) { + PyObject *bitarray_module; + + if ((bitarray_module = PyImport_ImportModule("bitarray")) == NULL) + return -1; + frozen = PyObject_GetAttrString(bitarray_module, "frozenbitarray"); + Py_DECREF(bitarray_module); + if (frozen == NULL) + return -1; + } + if ((is_frozen = PyObject_IsInstance((PyObject *) self, frozen)) < 0) + return -1; + + self->readonly = is_frozen; + return 0; +} + /************************************************************************** Implementation of bitarray methods **************************************************************************/ @@ -954,7 +979,9 @@ bitarray_copy(bitarrayobject *self) return NULL; memcpy(res->ob_item, self->ob_item, (size_t) Py_SIZE(self)); - res->readonly = self->readonly; + if (set_readonly(res) < 0) + return NULL; + return (PyObject *) res; } @@ -1837,13 +1864,14 @@ bitarray_concat(bitarrayobject *self, PyObject *other) if (res == NULL) return NULL; - res->readonly = 0; + res->readonly = 0; /* allow resize */ if (extend_dispatch(res, other) < 0) { Py_DECREF(res); return NULL; } set_padbits(res); - res->readonly = self->readonly; + if (set_readonly(res) < 0) + return NULL; return (PyObject *) res; } @@ -1857,13 +1885,14 @@ bitarray_repeat(bitarrayobject *self, Py_ssize_t n) if (res == NULL) return NULL; - res->readonly = 0; + res->readonly = 0; /* allow resize */ if (repeat(res, n) < 0) { Py_DECREF(res); return NULL; } set_padbits(res); - res->readonly = self->readonly; + if (set_readonly(res) < 0) + return NULL; return (PyObject *) res; } @@ -1968,7 +1997,8 @@ getslice(bitarrayobject *self, PyObject *slice) setbit(res, i, getbit(self, j)); } set_padbits(res); - res->readonly = self->readonly; + if (set_readonly(res) < 0) + return NULL; return (PyObject *) res; } @@ -2004,7 +2034,8 @@ getmasked(bitarrayobject *self, bitarrayobject *mask) setbit(res, j++, getbit(self, i)); } set_padbits(res); - res->readonly = self->readonly; + if (set_readonly(res) < 0) + return NULL; assert(j == n); return (PyObject *) res; } @@ -2055,7 +2086,8 @@ getsequence(bitarrayobject *self, PyObject *seq) setbit(res, j, getbit(self, i)); } set_padbits(res); - res->readonly = self->readonly; + if (set_readonly(res) < 0) + return NULL; return (PyObject *) res; } diff --git a/bitarray/test_bitarray.py b/bitarray/test_bitarray.py index 9ec91816..6ce2ce9b 100644 --- a/bitarray/test_bitarray.py +++ b/bitarray/test_bitarray.py @@ -4559,6 +4559,13 @@ def test_bitarray(self): self.check_obj(a) self.check_obj(b) + def test_copy(self): + a = bitarray(buffer=b'XA') + self.assertTrue(a.readonly) + b = a.copy() + self.assertFalse(b.readonly) + self.check_obj(b) + @skipIf(is_pypy) def test_bitarray_shared_sections(self): a = urandom(0x2000) From 58424b029571d008ea7f2e28d081532375465be6 Mon Sep 17 00:00:00 2001 From: Ilan Schnell Date: Thu, 9 Nov 2023 23:21:03 -0600 Subject: [PATCH 3/5] move set_readonly() --- bitarray/_bitarray.c | 51 ++++++++++++++++++++++---------------------- 1 file changed, 26 insertions(+), 25 deletions(-) diff --git a/bitarray/_bitarray.c b/bitarray/_bitarray.c index d1f4ea62..bc9cdfbe 100644 --- a/bitarray/_bitarray.c +++ b/bitarray/_bitarray.c @@ -784,31 +784,6 @@ extend_dispatch(bitarrayobject *self, PyObject *obj) return -1; } -/* set the readonly member to 0 or 1 depending on whether self in an instance - of the frozenbitarray class - on error, return -1 and set an exception. */ -static int -set_readonly(bitarrayobject *self) -{ - static PyObject *frozen = NULL; /* frozenbitarray class object */ - int is_frozen; - - if (frozen == NULL) { - PyObject *bitarray_module; - - if ((bitarray_module = PyImport_ImportModule("bitarray")) == NULL) - return -1; - frozen = PyObject_GetAttrString(bitarray_module, "frozenbitarray"); - Py_DECREF(bitarray_module); - if (frozen == NULL) - return -1; - } - if ((is_frozen = PyObject_IsInstance((PyObject *) self, frozen)) < 0) - return -1; - - self->readonly = is_frozen; - return 0; -} - /************************************************************************** Implementation of bitarray methods **************************************************************************/ @@ -969,6 +944,32 @@ PyDoc_STRVAR(clear_doc, Remove all items from the bitarray."); +/* Set the readonly member to 0 or 1 depending on whether self in an instance + of frozenbitarray. On error, return -1 and set an exception. */ +static int +set_readonly(bitarrayobject *self) +{ + static PyObject *frozen = NULL; /* frozenbitarray class object */ + int is_frozen; + + if (frozen == NULL) { + PyObject *bitarray_module; + + if ((bitarray_module = PyImport_ImportModule("bitarray")) == NULL) + return -1; + frozen = PyObject_GetAttrString(bitarray_module, "frozenbitarray"); + Py_DECREF(bitarray_module); + if (frozen == NULL) + return -1; + } + if ((is_frozen = PyObject_IsInstance((PyObject *) self, frozen)) < 0) + return -1; + + self->readonly = is_frozen; + return 0; +} + + static PyObject * bitarray_copy(bitarrayobject *self) { From 63f0fe1051a1fa88c63f684ef4c18ed597456fc2 Mon Sep 17 00:00:00 2001 From: Ilan Schnell Date: Thu, 9 Nov 2023 23:49:03 -0600 Subject: [PATCH 4/5] improve test --- bitarray/test_bitarray.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/bitarray/test_bitarray.py b/bitarray/test_bitarray.py index 6ce2ce9b..008c969a 100644 --- a/bitarray/test_bitarray.py +++ b/bitarray/test_bitarray.py @@ -4562,9 +4562,10 @@ def test_bitarray(self): def test_copy(self): a = bitarray(buffer=b'XA') self.assertTrue(a.readonly) - b = a.copy() - self.assertFalse(b.readonly) - self.check_obj(b) + for b in [a.copy(), 3 * a, 5 * a, a & bitarray(16), + a + bitarray(8*'1'), a[:], a[[0, 1]], a[bitarray(16)]]: + self.assertFalse(b.readonly) + self.check_obj(b) @skipIf(is_pypy) def test_bitarray_shared_sections(self): From 9b078be47e4567163c63b263eebe2521985474f0 Mon Sep 17 00:00:00 2001 From: Ilan Schnell Date: Fri, 10 Nov 2023 00:22:43 -0600 Subject: [PATCH 5/5] fix debug mode - add test --- bitarray/_bitarray.c | 4 ++++ bitarray/test_bitarray.py | 9 +++++++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/bitarray/_bitarray.c b/bitarray/_bitarray.c index bc9cdfbe..a825b24d 100644 --- a/bitarray/_bitarray.c +++ b/bitarray/_bitarray.c @@ -2634,10 +2634,14 @@ bitarray_ ## name (PyObject *self, PyObject *other) \ } \ else { \ res = bitarray_copy((bitarrayobject *) self); \ + ((bitarrayobject *) res)->readonly = 0; \ if (res == NULL) \ return NULL; \ } \ shift((bitarrayobject *) res, n, *ostr == '>'); \ + if (!inplace && \ + set_readonly((bitarrayobject *) res)) \ + return NULL; \ return res; \ } diff --git a/bitarray/test_bitarray.py b/bitarray/test_bitarray.py index 008c969a..b1bd7476 100644 --- a/bitarray/test_bitarray.py +++ b/bitarray/test_bitarray.py @@ -2601,6 +2601,11 @@ def test_shift_example(self): a >>= 4 self.assertEqual(a, bitarray('0000001')) + def test_frozenbitarray(self): + a = frozenbitarray('0010011') + self.assertEqual(a << 3, bitarray('0011000')) + self.assertRaises(TypeError, a.__ilshift__, 4) + # --------------------------------------------------------------------------- class ExtendTests(unittest.TestCase, Util): @@ -4562,7 +4567,7 @@ def test_bitarray(self): def test_copy(self): a = bitarray(buffer=b'XA') self.assertTrue(a.readonly) - for b in [a.copy(), 3 * a, 5 * a, a & bitarray(16), + for b in [a.copy(), 3 * a, 5 * a, a & bitarray(16), a >> 2, a + bitarray(8*'1'), a[:], a[[0, 1]], a[bitarray(16)]]: self.assertFalse(b.readonly) self.check_obj(b) @@ -4886,7 +4891,7 @@ def test_immutable(self): def test_copy(self): a = frozenbitarray('101') # not only .copy() creates new frozenbitarray which are read-only - for b in [a, a.copy(), 3 * a, 5 * a, a & bitarray('110'), + for b in [a, a.copy(), 3 * a, 5 * a, a & bitarray('110'), a >> 2, a + bitarray(8*'1'), a[:], a[[0, 1]], a[bitarray('011')]]: self.assertIsType(b, 'frozenbitarray') self.assertTrue(b.readonly)