From 42782b830ee3720645d830847c8266bfc67b7a17 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Sun, 11 May 2025 22:03:58 +0100 Subject: [PATCH 1/6] Convert dict_content to take Py_buffer --- Modules/_zstd/clinic/zstddict.c.h | 43 +++++++++++++++--- Modules/_zstd/compressor.c | 4 +- Modules/_zstd/decompressor.c | 16 +++---- Modules/_zstd/zstddict.c | 73 ++++++++++++++++--------------- Modules/_zstd/zstddict.h | 6 ++- 5 files changed, 87 insertions(+), 55 deletions(-) diff --git a/Modules/_zstd/clinic/zstddict.c.h b/Modules/_zstd/clinic/zstddict.c.h index 810befdaf71f44..1ee4bd3c6052d4 100644 --- a/Modules/_zstd/clinic/zstddict.c.h +++ b/Modules/_zstd/clinic/zstddict.c.h @@ -25,7 +25,7 @@ PyDoc_STRVAR(_zstd_ZstdDict_new__doc__, "by multiple ZstdCompressor or ZstdDecompressor objects."); static PyObject * -_zstd_ZstdDict_new_impl(PyTypeObject *type, PyObject *dict_content, +_zstd_ZstdDict_new_impl(PyTypeObject *type, Py_buffer *dict_content, int is_raw); static PyObject * @@ -63,7 +63,7 @@ _zstd_ZstdDict_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) PyObject * const *fastargs; Py_ssize_t nargs = PyTuple_GET_SIZE(args); Py_ssize_t noptargs = nargs + (kwargs ? PyDict_GET_SIZE(kwargs) : 0) - 1; - PyObject *dict_content; + Py_buffer dict_content = {NULL, NULL}; int is_raw = 0; fastargs = _PyArg_UnpackKeywords(_PyTuple_CAST(args)->ob_item, nargs, kwargs, NULL, &_parser, @@ -71,7 +71,9 @@ _zstd_ZstdDict_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) if (!fastargs) { goto exit; } - dict_content = fastargs[0]; + if (PyObject_GetBuffer(fastargs[0], &dict_content, PyBUF_SIMPLE) != 0) { + goto exit; + } if (!noptargs) { goto skip_optional_kwonly; } @@ -80,12 +82,43 @@ _zstd_ZstdDict_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) goto exit; } skip_optional_kwonly: - return_value = _zstd_ZstdDict_new_impl(type, dict_content, is_raw); + return_value = _zstd_ZstdDict_new_impl(type, &dict_content, is_raw); exit: + /* Cleanup for dict_content */ + if (dict_content.obj) { + PyBuffer_Release(&dict_content); + } + return return_value; } +PyDoc_STRVAR(_zstd_ZstdDict_dict_content__doc__, +"The content of a Zstandard dictionary, as a bytes object."); +#if defined(_zstd_ZstdDict_dict_content_DOCSTR) +# undef _zstd_ZstdDict_dict_content_DOCSTR +#endif +#define _zstd_ZstdDict_dict_content_DOCSTR _zstd_ZstdDict_dict_content__doc__ + +#if !defined(_zstd_ZstdDict_dict_content_DOCSTR) +# define _zstd_ZstdDict_dict_content_DOCSTR NULL +#endif +#if defined(_ZSTD_ZSTDDICT_DICT_CONTENT_GETSETDEF) +# undef _ZSTD_ZSTDDICT_DICT_CONTENT_GETSETDEF +# define _ZSTD_ZSTDDICT_DICT_CONTENT_GETSETDEF {"dict_content", (getter)_zstd_ZstdDict_dict_content_get, (setter)_zstd_ZstdDict_dict_content_set, _zstd_ZstdDict_dict_content_DOCSTR}, +#else +# define _ZSTD_ZSTDDICT_DICT_CONTENT_GETSETDEF {"dict_content", (getter)_zstd_ZstdDict_dict_content_get, NULL, _zstd_ZstdDict_dict_content_DOCSTR}, +#endif + +static PyObject * +_zstd_ZstdDict_dict_content_get_impl(ZstdDict *self); + +static PyObject * +_zstd_ZstdDict_dict_content_get(PyObject *self, void *Py_UNUSED(context)) +{ + return _zstd_ZstdDict_dict_content_get_impl((ZstdDict *)self); +} + PyDoc_STRVAR(_zstd_ZstdDict_as_digested_dict__doc__, "Load as a digested dictionary to compressor.\n" "\n" @@ -189,4 +222,4 @@ _zstd_ZstdDict_as_prefix_get(PyObject *self, void *Py_UNUSED(context)) { return _zstd_ZstdDict_as_prefix_get_impl((ZstdDict *)self); } -/*[clinic end generated code: output=47b12b5848b53ed8 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=0418adb04c9d85a7 input=a9049054013a1b77]*/ diff --git a/Modules/_zstd/compressor.c b/Modules/_zstd/compressor.c index 31cb8c535c05a6..6ff076cd76f83a 100644 --- a/Modules/_zstd/compressor.c +++ b/Modules/_zstd/compressor.c @@ -173,8 +173,8 @@ _get_CDict(ZstdDict *self, int compressionLevel) } if (capsule == NULL) { /* Create ZSTD_CDict instance */ - char *dict_buffer = PyBytes_AS_STRING(self->dict_content); - Py_ssize_t dict_len = Py_SIZE(self->dict_content); + char *dict_buffer = self->dict_buffer; + Py_ssize_t dict_len = self->dict_len; Py_BEGIN_ALLOW_THREADS cdict = ZSTD_createCDict(dict_buffer, dict_len, diff --git a/Modules/_zstd/decompressor.c b/Modules/_zstd/decompressor.c index d084f0847c72dd..7ab7067ef1ee05 100644 --- a/Modules/_zstd/decompressor.c +++ b/Modules/_zstd/decompressor.c @@ -68,8 +68,8 @@ _get_DDict(ZstdDict *self) if (self->d_dict == NULL) { /* Create ZSTD_DDict instance from dictionary content */ - char *dict_buffer = PyBytes_AS_STRING(self->dict_content); - Py_ssize_t dict_len = Py_SIZE(self->dict_content); + char *dict_buffer = self->dict_buffer; + Py_ssize_t dict_len = self->dict_len; Py_BEGIN_ALLOW_THREADS ret = ZSTD_createDDict(dict_buffer, dict_len); Py_END_ALLOW_THREADS @@ -160,17 +160,13 @@ _zstd_load_impl(ZstdDecompressor *self, ZstdDict *zd, } else if (type == DICT_TYPE_UNDIGESTED) { /* Load a dictionary */ - zstd_ret = ZSTD_DCtx_loadDictionary( - self->dctx, - PyBytes_AS_STRING(zd->dict_content), - Py_SIZE(zd->dict_content)); + zstd_ret = ZSTD_DCtx_loadDictionary(self->dctx, zd->dict_buffer, + zd->dict_len); } else if (type == DICT_TYPE_PREFIX) { /* Load a prefix */ - zstd_ret = ZSTD_DCtx_refPrefix( - self->dctx, - PyBytes_AS_STRING(zd->dict_content), - Py_SIZE(zd->dict_content)); + zstd_ret = ZSTD_DCtx_refPrefix(self->dctx, zd->dict_buffer, + zd->dict_len); } else { /* Impossible code path */ diff --git a/Modules/_zstd/zstddict.c b/Modules/_zstd/zstddict.c index e3e9e5d064515f..a8b136ee25d4f5 100644 --- a/Modules/_zstd/zstddict.c +++ b/Modules/_zstd/zstddict.c @@ -26,7 +26,7 @@ class _zstd.ZstdDict "ZstdDict *" "&zstd_dict_type_spec" /*[clinic input] @classmethod _zstd.ZstdDict.__new__ as _zstd_ZstdDict_new - dict_content: object + dict_content: Py_buffer The content of a Zstandard dictionary as a bytes-like object. / * @@ -42,16 +42,15 @@ by multiple ZstdCompressor or ZstdDecompressor objects. [clinic start generated code]*/ static PyObject * -_zstd_ZstdDict_new_impl(PyTypeObject *type, PyObject *dict_content, +_zstd_ZstdDict_new_impl(PyTypeObject *type, Py_buffer *dict_content, int is_raw) -/*[clinic end generated code: output=3ebff839cb3be6d7 input=6b5de413869ae878]*/ +/*[clinic end generated code: output=685b7406a48b0949 input=9e8c493e31c98383]*/ { ZstdDict* self = PyObject_GC_New(ZstdDict, type); if (self == NULL) { - goto error; + return NULL; } - self->dict_content = NULL; self->d_dict = NULL; self->dict_id = 0; self->lock = (PyMutex){0}; @@ -63,16 +62,21 @@ _zstd_ZstdDict_new_impl(PyTypeObject *type, PyObject *dict_content, } /* Check dict_content's type */ - self->dict_content = PyBytes_FromObject(dict_content); - if (self->dict_content == NULL) { + if (dict_content == NULL) { PyErr_SetString(PyExc_TypeError, "dict_content argument should be bytes-like object."); goto error; } - /* Both ordinary dictionary and "raw content" dictionary should - at least 8 bytes */ - if (Py_SIZE(self->dict_content) < 8) { + self->dict_buffer = PyMem_RawMalloc(dict_content->len); + if (!self->dict_buffer) { + return PyErr_NoMemory(); + } + memcpy(self->dict_buffer, dict_content->buf, dict_content->len); + self->dict_len = dict_content->len; + + /* Both ordinary and "raw content" dictionaries must be 8 bytes minimum */ + if (self->dict_len < 8) { PyErr_SetString(PyExc_ValueError, "Zstandard dictionary content should at least " "8 bytes."); @@ -81,8 +85,7 @@ _zstd_ZstdDict_new_impl(PyTypeObject *type, PyObject *dict_content, /* Get dict_id, 0 means "raw content" dictionary. */ self->dict_id = ZSTD_getDictID_fromDict( - PyBytes_AS_STRING(self->dict_content), - Py_SIZE(self->dict_content)); + self->dict_buffer, self->dict_len); /* Check validity for ordinary dictionary */ if (!is_raw && self->dict_id == 0) { @@ -91,13 +94,13 @@ _zstd_ZstdDict_new_impl(PyTypeObject *type, PyObject *dict_content, goto error; } - // Can only track self once self->dict_content is included PyObject_GC_Track(self); return (PyObject*)self; error: Py_XDECREF(self); + PyObject_GC_Del(self); return NULL; } @@ -115,12 +118,12 @@ ZstdDict_dealloc(PyObject *ob) assert(!PyMutex_IsLocked(&self->lock)); - /* Release dict_content after Free ZSTD_CDict/ZSTD_DDict instances */ - Py_CLEAR(self->dict_content); + /* Release dict_buffer after Free ZSTD_CDict/ZSTD_DDict instances */ + PyMem_RawFree(self->dict_buffer); Py_CLEAR(self->c_dicts); PyTypeObject *tp = Py_TYPE(self); - PyObject_GC_Del(ob); + tp->tp_free(self); Py_DECREF(tp); } @@ -131,25 +134,33 @@ PyDoc_STRVAR(ZstdDict_dictid_doc, "The special value '0' means a 'raw content' dictionary," "without any restrictions on format or content."); -PyDoc_STRVAR(ZstdDict_dictcontent_doc, -"The content of a Zstandard dictionary, as a bytes object."); - static PyObject * ZstdDict_str(PyObject *ob) { ZstdDict *dict = ZstdDict_CAST(ob); return PyUnicode_FromFormat("", - dict->dict_id, Py_SIZE(dict->dict_content)); + dict->dict_id, dict->dict_len); } static PyMemberDef ZstdDict_members[] = { - {"dict_id", Py_T_UINT, offsetof(ZstdDict, dict_id), Py_READONLY, - ZstdDict_dictid_doc}, - {"dict_content", Py_T_OBJECT_EX, offsetof(ZstdDict, dict_content), - Py_READONLY, ZstdDict_dictcontent_doc}, + {"dict_id", Py_T_UINT, offsetof(ZstdDict, dict_id), Py_READONLY, ZstdDict_dictid_doc}, {NULL} }; +/*[clinic input] +@getter +_zstd.ZstdDict.dict_content + +The content of a Zstandard dictionary, as a bytes object. +[clinic start generated code]*/ + +static PyObject * +_zstd_ZstdDict_dict_content_get_impl(ZstdDict *self) +/*[clinic end generated code: output=0d05caa5b550eabb input=4ed526d1c151c596]*/ +{ + return PyBytes_FromStringAndSize(self->dict_buffer, self->dict_len); +} + /*[clinic input] @getter _zstd.ZstdDict.as_digested_dict @@ -219,6 +230,7 @@ _zstd_ZstdDict_as_prefix_get_impl(ZstdDict *self) } static PyGetSetDef ZstdDict_getset[] = { + _ZSTD_ZSTDDICT_DICT_CONTENT_GETSETDEF _ZSTD_ZSTDDICT_AS_DIGESTED_DICT_GETSETDEF _ZSTD_ZSTDDICT_AS_UNDIGESTED_DICT_GETSETDEF _ZSTD_ZSTDDICT_AS_PREFIX_GETSETDEF @@ -229,8 +241,7 @@ static Py_ssize_t ZstdDict_length(PyObject *ob) { ZstdDict *self = ZstdDict_CAST(ob); - assert(PyBytes_Check(self->dict_content)); - return Py_SIZE(self->dict_content); + return self->dict_len; } static int @@ -238,15 +249,6 @@ ZstdDict_traverse(PyObject *ob, visitproc visit, void *arg) { ZstdDict *self = ZstdDict_CAST(ob); Py_VISIT(self->c_dicts); - Py_VISIT(self->dict_content); - return 0; -} - -static int -ZstdDict_clear(PyObject *ob) -{ - ZstdDict *self = ZstdDict_CAST(ob); - Py_CLEAR(self->dict_content); return 0; } @@ -259,7 +261,6 @@ static PyType_Slot zstddict_slots[] = { {Py_tp_doc, (void *)_zstd_ZstdDict_new__doc__}, {Py_sq_length, ZstdDict_length}, {Py_tp_traverse, ZstdDict_traverse}, - {Py_tp_clear, ZstdDict_clear}, {0, 0} }; diff --git a/Modules/_zstd/zstddict.h b/Modules/_zstd/zstddict.h index dcba0f21852087..4a403416dbd4a3 100644 --- a/Modules/_zstd/zstddict.h +++ b/Modules/_zstd/zstddict.h @@ -15,8 +15,10 @@ typedef struct { ZSTD_DDict *d_dict; PyObject *c_dicts; - /* Content of the dictionary, bytes object. */ - PyObject *dict_content; + /* Dictionary content. */ + char *dict_buffer; + Py_ssize_t dict_len; + /* Dictionary id */ uint32_t dict_id; From 7c0d002de89f2d4fb64db635e0fb02d18e893b8f Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Sat, 24 May 2025 08:32:17 +0100 Subject: [PATCH 2/6] Respond to review comments --- Modules/_zstd/clinic/zstddict.c.h | 2 +- Modules/_zstd/compressor.c | 17 +++------- Modules/_zstd/decompressor.c | 4 +-- Modules/_zstd/zstddict.c | 53 ++++++++++++++++--------------- 4 files changed, 34 insertions(+), 42 deletions(-) diff --git a/Modules/_zstd/clinic/zstddict.c.h b/Modules/_zstd/clinic/zstddict.c.h index 1ee4bd3c6052d4..79db85405d6e6b 100644 --- a/Modules/_zstd/clinic/zstddict.c.h +++ b/Modules/_zstd/clinic/zstddict.c.h @@ -222,4 +222,4 @@ _zstd_ZstdDict_as_prefix_get(PyObject *self, void *Py_UNUSED(context)) { return _zstd_ZstdDict_as_prefix_get_impl((ZstdDict *)self); } -/*[clinic end generated code: output=0418adb04c9d85a7 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=4696cbc722e5fdfc input=a9049054013a1b77]*/ diff --git a/Modules/_zstd/compressor.c b/Modules/_zstd/compressor.c index 6ff076cd76f83a..7f0558909b4422 100644 --- a/Modules/_zstd/compressor.c +++ b/Modules/_zstd/compressor.c @@ -173,11 +173,8 @@ _get_CDict(ZstdDict *self, int compressionLevel) } if (capsule == NULL) { /* Create ZSTD_CDict instance */ - char *dict_buffer = self->dict_buffer; - Py_ssize_t dict_len = self->dict_len; Py_BEGIN_ALLOW_THREADS - cdict = ZSTD_createCDict(dict_buffer, - dict_len, + cdict = ZSTD_createCDict(self->dict_buffer, self->dict_len, compressionLevel); Py_END_ALLOW_THREADS @@ -236,17 +233,13 @@ _zstd_load_impl(ZstdCompressor *self, ZstdDict *zd, else if (type == DICT_TYPE_UNDIGESTED) { /* Load a dictionary. It doesn't override compression context's parameters. */ - zstd_ret = ZSTD_CCtx_loadDictionary( - self->cctx, - PyBytes_AS_STRING(zd->dict_content), - Py_SIZE(zd->dict_content)); + zstd_ret = ZSTD_CCtx_loadDictionary(self->cctx, zd->dict_buffer, + zd->dict_len); } else if (type == DICT_TYPE_PREFIX) { /* Load a prefix */ - zstd_ret = ZSTD_CCtx_refPrefix( - self->cctx, - PyBytes_AS_STRING(zd->dict_content), - Py_SIZE(zd->dict_content)); + zstd_ret = ZSTD_CCtx_refPrefix(self->cctx, zd->dict_buffer, + zd->dict_len); } else { Py_UNREACHABLE(); diff --git a/Modules/_zstd/decompressor.c b/Modules/_zstd/decompressor.c index 7ab7067ef1ee05..015cb774ed2d76 100644 --- a/Modules/_zstd/decompressor.c +++ b/Modules/_zstd/decompressor.c @@ -68,10 +68,8 @@ _get_DDict(ZstdDict *self) if (self->d_dict == NULL) { /* Create ZSTD_DDict instance from dictionary content */ - char *dict_buffer = self->dict_buffer; - Py_ssize_t dict_len = self->dict_len; Py_BEGIN_ALLOW_THREADS - ret = ZSTD_createDDict(dict_buffer, dict_len); + ret = ZSTD_createDDict(self->dict_buffer, self->dict_len); Py_END_ALLOW_THREADS self->d_dict = ret; diff --git a/Modules/_zstd/zstddict.c b/Modules/_zstd/zstddict.c index a8b136ee25d4f5..e8738b064dceac 100644 --- a/Modules/_zstd/zstddict.c +++ b/Modules/_zstd/zstddict.c @@ -46,12 +46,21 @@ _zstd_ZstdDict_new_impl(PyTypeObject *type, Py_buffer *dict_content, int is_raw) /*[clinic end generated code: output=685b7406a48b0949 input=9e8c493e31c98383]*/ { + /* All dictionaries must be at least 8 bytes */ + if (dict_content->len < 8) { + PyErr_SetString(PyExc_ValueError, + "Zstandard dictionary content should at least " + "8 bytes."); + goto error; + } + ZstdDict* self = PyObject_GC_New(ZstdDict, type); if (self == NULL) { return NULL; } self->d_dict = NULL; + self->dict_buffer = NULL; self->dict_id = 0; self->lock = (PyMutex){0}; @@ -61,36 +70,20 @@ _zstd_ZstdDict_new_impl(PyTypeObject *type, Py_buffer *dict_content, goto error; } - /* Check dict_content's type */ - if (dict_content == NULL) { - PyErr_SetString(PyExc_TypeError, - "dict_content argument should be bytes-like object."); - goto error; - } - - self->dict_buffer = PyMem_RawMalloc(dict_content->len); + self->dict_buffer = PyMem_Malloc(dict_content->len); if (!self->dict_buffer) { + Py_DECREF(self); return PyErr_NoMemory(); } memcpy(self->dict_buffer, dict_content->buf, dict_content->len); self->dict_len = dict_content->len; - /* Both ordinary and "raw content" dictionaries must be 8 bytes minimum */ - if (self->dict_len < 8) { - PyErr_SetString(PyExc_ValueError, - "Zstandard dictionary content should at least " - "8 bytes."); - goto error; - } - /* Get dict_id, 0 means "raw content" dictionary. */ - self->dict_id = ZSTD_getDictID_fromDict( - self->dict_buffer, self->dict_len); + self->dict_id = ZSTD_getDictID_fromDict(self->dict_buffer, self->dict_len); /* Check validity for ordinary dictionary */ if (!is_raw && self->dict_id == 0) { - char *msg = "Invalid Zstandard dictionary and is_raw not set.\n"; - PyErr_SetString(PyExc_ValueError, msg); + PyErr_SetString(PyExc_ValueError, "invalid Zstandard dictionary."); goto error; } @@ -100,7 +93,6 @@ _zstd_ZstdDict_new_impl(PyTypeObject *type, Py_buffer *dict_content, error: Py_XDECREF(self); - PyObject_GC_Del(self); return NULL; } @@ -118,8 +110,8 @@ ZstdDict_dealloc(PyObject *ob) assert(!PyMutex_IsLocked(&self->lock)); - /* Release dict_buffer after Free ZSTD_CDict/ZSTD_DDict instances */ - PyMem_RawFree(self->dict_buffer); + /* Release dict_buffer after freeing ZSTD_CDict/ZSTD_DDict instances */ + PyMem_Free(self->dict_buffer); Py_CLEAR(self->c_dicts); PyTypeObject *tp = Py_TYPE(self); @@ -135,11 +127,11 @@ PyDoc_STRVAR(ZstdDict_dictid_doc, "without any restrictions on format or content."); static PyObject * -ZstdDict_str(PyObject *ob) +ZstdDict_repr(PyObject *ob) { ZstdDict *dict = ZstdDict_CAST(ob); return PyUnicode_FromFormat("", - dict->dict_id, dict->dict_len); + (unsigned int)dict->dict_id, dict->dict_len); } static PyMemberDef ZstdDict_members[] = { @@ -252,15 +244,24 @@ ZstdDict_traverse(PyObject *ob, visitproc visit, void *arg) return 0; } +static int +ZstdDict_clear(PyObject *ob) +{ + ZstdDict *self = ZstdDict_CAST(ob); + Py_CLEAR(self->c_dicts); + return 0; +} + static PyType_Slot zstddict_slots[] = { {Py_tp_members, ZstdDict_members}, {Py_tp_getset, ZstdDict_getset}, {Py_tp_new, _zstd_ZstdDict_new}, {Py_tp_dealloc, ZstdDict_dealloc}, - {Py_tp_str, ZstdDict_str}, + {Py_tp_repr, ZstdDict_repr}, {Py_tp_doc, (void *)_zstd_ZstdDict_new__doc__}, {Py_sq_length, ZstdDict_length}, {Py_tp_traverse, ZstdDict_traverse}, + {Py_tp_clear, ZstdDict_clear}, {0, 0} }; From 30ac9d5129d6d4075a5a672a5d7fc7f62624544e Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Mon, 26 May 2025 08:50:34 +0100 Subject: [PATCH 3/6] Respond to review comments (2) --- Modules/_zstd/zstddict.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Modules/_zstd/zstddict.c b/Modules/_zstd/zstddict.c index e8738b064dceac..9c60bec910615e 100644 --- a/Modules/_zstd/zstddict.c +++ b/Modules/_zstd/zstddict.c @@ -49,8 +49,8 @@ _zstd_ZstdDict_new_impl(PyTypeObject *type, Py_buffer *dict_content, /* All dictionaries must be at least 8 bytes */ if (dict_content->len < 8) { PyErr_SetString(PyExc_ValueError, - "Zstandard dictionary content should at least " - "8 bytes."); + "Zstandard dictionary content must be longer " + "than eight bytes."); goto error; } @@ -72,8 +72,8 @@ _zstd_ZstdDict_new_impl(PyTypeObject *type, Py_buffer *dict_content, self->dict_buffer = PyMem_Malloc(dict_content->len); if (!self->dict_buffer) { - Py_DECREF(self); - return PyErr_NoMemory(); + PyErr_NoMemory(); + goto error; } memcpy(self->dict_buffer, dict_content->buf, dict_content->len); self->dict_len = dict_content->len; From e40de1500470691f06fa28386393fe828295091b Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Mon, 26 May 2025 09:02:37 +0100 Subject: [PATCH 4/6] fixup! Respond to review comments (2) --- Modules/_zstd/zstddict.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_zstd/zstddict.c b/Modules/_zstd/zstddict.c index 9c60bec910615e..fc9f45165ce243 100644 --- a/Modules/_zstd/zstddict.c +++ b/Modules/_zstd/zstddict.c @@ -51,7 +51,7 @@ _zstd_ZstdDict_new_impl(PyTypeObject *type, Py_buffer *dict_content, PyErr_SetString(PyExc_ValueError, "Zstandard dictionary content must be longer " "than eight bytes."); - goto error; + return NULL; } ZstdDict* self = PyObject_GC_New(ZstdDict, type); From feb034df5833604fbe0374896b2cebc877db8dbc Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Mon, 26 May 2025 12:59:56 +0100 Subject: [PATCH 5/6] Improve error --- Modules/_zstd/zstddict.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Modules/_zstd/zstddict.c b/Modules/_zstd/zstddict.c index fc9f45165ce243..2e7834394119ef 100644 --- a/Modules/_zstd/zstddict.c +++ b/Modules/_zstd/zstddict.c @@ -49,8 +49,8 @@ _zstd_ZstdDict_new_impl(PyTypeObject *type, Py_buffer *dict_content, /* All dictionaries must be at least 8 bytes */ if (dict_content->len < 8) { PyErr_SetString(PyExc_ValueError, - "Zstandard dictionary content must be longer " - "than eight bytes."); + "Zstandard dictionary content too short " + "(must have at least eight bytes)"); return NULL; } @@ -89,7 +89,7 @@ _zstd_ZstdDict_new_impl(PyTypeObject *type, Py_buffer *dict_content, PyObject_GC_Track(self); - return (PyObject*)self; + return (PyObject *)self; error: Py_XDECREF(self); From c36f62dbf4445040af7aaf02b1c6309cdc40aa5d Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Mon, 26 May 2025 15:22:42 +0100 Subject: [PATCH 6/6] delete full stop --- Modules/_zstd/zstddict.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_zstd/zstddict.c b/Modules/_zstd/zstddict.c index 2e7834394119ef..afc58b42e893d3 100644 --- a/Modules/_zstd/zstddict.c +++ b/Modules/_zstd/zstddict.c @@ -83,7 +83,7 @@ _zstd_ZstdDict_new_impl(PyTypeObject *type, Py_buffer *dict_content, /* Check validity for ordinary dictionary */ if (!is_raw && self->dict_id == 0) { - PyErr_SetString(PyExc_ValueError, "invalid Zstandard dictionary."); + PyErr_SetString(PyExc_ValueError, "invalid Zstandard dictionary"); goto error; }