Skip to content

Commit fe3fb49

Browse files
AA-TurnerAA-Turner
committed
Respond to review comments
1 parent 03b7113 commit fe3fb49

File tree

4 files changed

+34
-42
lines changed

4 files changed

+34
-42
lines changed

‎Modules/_zstd/clinic/zstddict.c.h

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Modules/_zstd/compressor.c

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -173,11 +173,8 @@ _get_CDict(ZstdDict *self, int compressionLevel)
173173
}
174174
if (capsule == NULL) {
175175
/* Create ZSTD_CDict instance */
176-
char *dict_buffer = self->dict_buffer;
177-
Py_ssize_t dict_len = self->dict_len;
178176
Py_BEGIN_ALLOW_THREADS
179-
cdict = ZSTD_createCDict(dict_buffer,
180-
dict_len,
177+
cdict = ZSTD_createCDict(self->dict_buffer, self->dict_len,
181178
compressionLevel);
182179
Py_END_ALLOW_THREADS
183180

@@ -236,17 +233,13 @@ _zstd_load_impl(ZstdCompressor *self, ZstdDict *zd,
236233
else if (type == DICT_TYPE_UNDIGESTED) {
237234
/* Load a dictionary.
238235
It doesn't override compression context's parameters. */
239-
zstd_ret = ZSTD_CCtx_loadDictionary(
240-
self->cctx,
241-
PyBytes_AS_STRING(zd->dict_content),
242-
Py_SIZE(zd->dict_content));
236+
zstd_ret = ZSTD_CCtx_loadDictionary(self->cctx, zd->dict_buffer,
237+
zd->dict_len);
243238
}
244239
else if (type == DICT_TYPE_PREFIX) {
245240
/* Load a prefix */
246-
zstd_ret = ZSTD_CCtx_refPrefix(
247-
self->cctx,
248-
PyBytes_AS_STRING(zd->dict_content),
249-
Py_SIZE(zd->dict_content));
241+
zstd_ret = ZSTD_CCtx_refPrefix(self->cctx, zd->dict_buffer,
242+
zd->dict_len);
250243
}
251244
else {
252245
Py_UNREACHABLE();

‎Modules/_zstd/decompressor.c

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,10 +68,8 @@ _get_DDict(ZstdDict *self)
6868

6969
if (self->d_dict == NULL) {
7070
/* Create ZSTD_DDict instance from dictionary content */
71-
char *dict_buffer = self->dict_buffer;
72-
Py_ssize_t dict_len = self->dict_len;
7371
Py_BEGIN_ALLOW_THREADS
74-
ret = ZSTD_createDDict(dict_buffer, dict_len);
72+
ret = ZSTD_createDDict(self->dict_buffer, self->dict_len);
7573
Py_END_ALLOW_THREADS
7674
self->d_dict = ret;
7775

‎Modules/_zstd/zstddict.c

Lines changed: 27 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -46,12 +46,21 @@ _zstd_ZstdDict_new_impl(PyTypeObject *type, Py_buffer *dict_content,
4646
int is_raw)
4747
/*[clinic end generated code: output=685b7406a48b0949 input=9e8c493e31c98383]*/
4848
{
49+
/* All dictionaries must be at least 8 bytes */
50+
if (dict_content->len < 8) {
51+
PyErr_SetString(PyExc_ValueError,
52+
"Zstandard dictionary content should at least "
53+
"8 bytes.");
54+
goto error;
55+
}
56+
4957
ZstdDict* self = PyObject_GC_New(ZstdDict, type);
5058
if (self == NULL) {
5159
return NULL;
5260
}
5361

5462
self->d_dict = NULL;
63+
self->dict_buffer = NULL;
5564
self->dict_id = 0;
5665
self->lock = (PyMutex){0};
5766

@@ -61,36 +70,20 @@ _zstd_ZstdDict_new_impl(PyTypeObject *type, Py_buffer *dict_content,
6170
goto error;
6271
}
6372

64-
/* Check dict_content's type */
65-
if (dict_content == NULL) {
66-
PyErr_SetString(PyExc_TypeError,
67-
"dict_content argument should be bytes-like object.");
68-
goto error;
69-
}
70-
71-
self->dict_buffer = PyMem_RawMalloc(dict_content->len);
73+
self->dict_buffer = PyMem_Malloc(dict_content->len);
7274
if (!self->dict_buffer) {
75+
Py_DECREF(self);
7376
return PyErr_NoMemory();
7477
}
7578
memcpy(self->dict_buffer, dict_content->buf, dict_content->len);
7679
self->dict_len = dict_content->len;
7780

78-
/* Both ordinary and "raw content" dictionaries must be 8 bytes minimum */
79-
if (self->dict_len < 8) {
80-
PyErr_SetString(PyExc_ValueError,
81-
"Zstandard dictionary content should at least "
82-
"8 bytes.");
83-
goto error;
84-
}
85-
8681
/* Get dict_id, 0 means "raw content" dictionary. */
87-
self->dict_id = ZSTD_getDictID_fromDict(
88-
self->dict_buffer, self->dict_len);
82+
self->dict_id = ZSTD_getDictID_fromDict(self->dict_buffer, self->dict_len);
8983

9084
/* Check validity for ordinary dictionary */
9185
if (!is_raw && self->dict_id == 0) {
92-
char *msg = "Invalid Zstandard dictionary and is_raw not set.\n";
93-
PyErr_SetString(PyExc_ValueError, msg);
86+
PyErr_SetString(PyExc_ValueError, "invalid Zstandard dictionary.");
9487
goto error;
9588
}
9689

@@ -100,7 +93,6 @@ _zstd_ZstdDict_new_impl(PyTypeObject *type, Py_buffer *dict_content,
10093

10194
error:
10295
Py_XDECREF(self);
103-
PyObject_GC_Del(self);
10496
return NULL;
10597
}
10698

@@ -118,8 +110,8 @@ ZstdDict_dealloc(PyObject *ob)
118110

119111
assert(!PyMutex_IsLocked(&self->lock));
120112

121-
/* Release dict_buffer after Free ZSTD_CDict/ZSTD_DDict instances */
122-
PyMem_RawFree(self->dict_buffer);
113+
/* Release dict_buffer after freeing ZSTD_CDict/ZSTD_DDict instances */
114+
PyMem_Free(self->dict_buffer);
123115
Py_CLEAR(self->c_dicts);
124116

125117
PyTypeObject *tp = Py_TYPE(self);
@@ -135,11 +127,11 @@ PyDoc_STRVAR(ZstdDict_dictid_doc,
135127
"without any restrictions on format or content.");
136128

137129
static PyObject *
138-
ZstdDict_str(PyObject *ob)
130+
ZstdDict_repr(PyObject *ob)
139131
{
140132
ZstdDict *dict = ZstdDict_CAST(ob);
141133
return PyUnicode_FromFormat("<ZstdDict dict_id=%u dict_size=%zd>",
142-
dict->dict_id, dict->dict_len);
134+
(unsigned int)dict->dict_id, dict->dict_len);
143135
}
144136

145137
static PyMemberDef ZstdDict_members[] = {
@@ -252,15 +244,24 @@ ZstdDict_traverse(PyObject *ob, visitproc visit, void *arg)
252244
return 0;
253245
}
254246

247+
static int
248+
ZstdDict_clear(PyObject *ob)
249+
{
250+
ZstdDict *self = ZstdDict_CAST(ob);
251+
Py_CLEAR(self->c_dicts);
252+
return 0;
253+
}
254+
255255
static PyType_Slot zstddict_slots[] = {
256256
{Py_tp_members, ZstdDict_members},
257257
{Py_tp_getset, ZstdDict_getset},
258258
{Py_tp_new, _zstd_ZstdDict_new},
259259
{Py_tp_dealloc, ZstdDict_dealloc},
260-
{Py_tp_str, ZstdDict_str},
260+
{Py_tp_repr, ZstdDict_repr},
261261
{Py_tp_doc, (void *)_zstd_ZstdDict_new__doc__},
262262
{Py_sq_length, ZstdDict_length},
263263
{Py_tp_traverse, ZstdDict_traverse},
264+
{Py_tp_clear, ZstdDict_clear},
264265
{0, 0}
265266
};
266267

0 commit comments

Comments
 (0)