Skip to content

Commit 5de9842

Browse files
tim-onetim-one
committed
Repair widespread misuse of _PyString_Resize. Since it's clear people
don't understand how this function works, also beefed up the docs. The most common usage error is of this form (often spread out across gotos): if (_PyString_Resize(&s, n) < 0) { Py_DECREF(s); s = NULL; goto outtahere; } The error is that if _PyString_Resize runs out of memory, it automatically decrefs the input string object s (which also deallocates it, since its refcount must be 1 upon entry), and sets s to NULL. So if the "if" branch ever triggers, it's an error to call Py_DECREF(s): s is already NULL! A correct way to write the above is the simpler (and intended) if (_PyString_Resize(&s, n) < 0) goto outtahere; Bugfix candidate.
1 parent 602f740 commit 5de9842

File tree

14 files changed

+54
-90
lines changed

14 files changed

+54
-90
lines changed

‎Doc/api/concrete.tex‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -586,7 +586,16 @@ \subsection{String Objects \label{stringObjects}}
586586
\begin{cfuncdesc}{int}{_PyString_Resize}{PyObject **string, int newsize}
587587
A way to resize a string object even though it is ``immutable''.
588588
Only use this to build up a brand new string object; don't use this
589-
if the string may already be known in other parts of the code.
589+
if the string may already be known in other parts of the code. It
590+
is an error to call this function if the refcount on the input string
591+
object is not one.
592+
Pass the address of an existing string object as an lvalue (it may
593+
be written into), and the new size desired. On success, \var{*string}
594+
holds the resized string object and 0 is returned; the address in
595+
\var{*string} may differ from its input value. If the
596+
reallocation fails, the original string object at \var{*string} is
597+
deallocated, \var{*string} is set to \NULL{}, a memory exception is set,
598+
and -1 is returned.
590599
\end{cfuncdesc}
591600

592601
\begin{cfuncdesc}{PyObject*}{PyString_Format}{PyObject *format,

‎Modules/_ssl.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -331,8 +331,8 @@ static PyObject *PySSL_SSLread(PySSLObject *self, PyObject *args)
331331
Py_DECREF(buf);
332332
return PySSL_SetError(self, count);
333333
}
334-
if (count != len && _PyString_Resize(&buf, count) < 0)
335-
return NULL;
334+
if (count != len)
335+
_PyString_Resize(&buf, count);
336336
return buf;
337337
}
338338

‎Modules/cPickle.c‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1252,14 +1252,8 @@ modified_EncodeRawUnicodeEscape(const Py_UNICODE *s, int size)
12521252
*p++ = (char) ch;
12531253
}
12541254
*p = '\0';
1255-
if (_PyString_Resize(&repr, p - q))
1256-
goto onError;
1257-
1255+
_PyString_Resize(&repr, p - q);
12581256
return repr;
1259-
1260-
onError:
1261-
Py_DECREF(repr);
1262-
return NULL;
12631257
}
12641258

12651259

‎Modules/cdmodule.c‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -251,8 +251,7 @@ CD_readda(cdplayerobject *self, PyObject *args)
251251
return NULL;
252252
}
253253
if (n < numframes)
254-
if (_PyString_Resize(&result, n * sizeof(CDFRAME)))
255-
return NULL;
254+
_PyString_Resize(&result, n * sizeof(CDFRAME));
256255

257256
return result;
258257
}

‎Modules/clmodule.c‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,8 +135,7 @@ cl_CompressImage(PyObject *self, PyObject *args)
135135
}
136136

137137
if (compressedBufferSize < frameBufferSize)
138-
if (_PyString_Resize(&compressedBuffer, compressedBufferSize))
139-
return NULL;
138+
_PyString_Resize(&compressedBuffer, compressedBufferSize);
140139

141140
return compressedBuffer;
142141
}

‎Modules/linuxaudiodev.c‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -158,8 +158,7 @@ lad_read(lad_t *self, PyObject *args)
158158
return NULL;
159159
}
160160
self->x_icount += count;
161-
if (_PyString_Resize(&rv, count) == -1)
162-
return NULL;
161+
_PyString_Resize(&rv, count);
163162
return rv;
164163
}
165164

‎Modules/regexmodule.c‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -516,11 +516,8 @@ symcomp(PyObject *pattern, PyObject *gdict)
516516
return NULL;
517517
}
518518
/* _PyString_Resize() decrements npattern on failure */
519-
if (_PyString_Resize(&npattern, n - v) == 0)
520-
return npattern;
521-
else {
522-
return NULL;
523-
}
519+
_PyString_Resize(&npattern, n - v);
520+
return npattern;
524521

525522
}
526523

‎Modules/socketmodule.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1402,8 +1402,8 @@ PySocketSock_recv(PySocketSockObject *s, PyObject *args)
14021402
Py_DECREF(buf);
14031403
return s->errorhandler();
14041404
}
1405-
if (n != len && _PyString_Resize(&buf, n) < 0)
1406-
return NULL;
1405+
if (n != len)
1406+
_PyString_Resize(&buf, n);
14071407
return buf;
14081408
}
14091409

‎Modules/stropmodule.c‎

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -215,10 +215,8 @@ strop_joinfields(PyObject *self, PyObject *args)
215215
}
216216
slen = PyString_GET_SIZE(item);
217217
while (reslen + slen + seplen >= sz) {
218-
if (_PyString_Resize(&res, sz * 2)) {
219-
Py_DECREF(res);
218+
if (_PyString_Resize(&res, sz * 2) < 0)
220219
return NULL;
221-
}
222220
sz *= 2;
223221
p = PyString_AsString(res) + reslen;
224222
}
@@ -231,10 +229,7 @@ strop_joinfields(PyObject *self, PyObject *args)
231229
p += slen;
232230
reslen += slen;
233231
}
234-
if (_PyString_Resize(&res, reslen)) {
235-
Py_DECREF(res);
236-
res = NULL;
237-
}
232+
_PyString_Resize(&res, reslen);
238233
return res;
239234
}
240235

@@ -257,8 +252,7 @@ strop_joinfields(PyObject *self, PyObject *args)
257252
}
258253
slen = PyString_GET_SIZE(item);
259254
while (reslen + slen + seplen >= sz) {
260-
if (_PyString_Resize(&res, sz * 2)) {
261-
Py_DECREF(res);
255+
if (_PyString_Resize(&res, sz * 2) < 0) {
262256
Py_DECREF(item);
263257
return NULL;
264258
}
@@ -275,10 +269,7 @@ strop_joinfields(PyObject *self, PyObject *args)
275269
reslen += slen;
276270
Py_DECREF(item);
277271
}
278-
if (_PyString_Resize(&res, reslen)) {
279-
Py_DECREF(res);
280-
res = NULL;
281-
}
272+
_PyString_Resize(&res, reslen);
282273
return res;
283274
}
284275

@@ -989,8 +980,8 @@ strop_translate(PyObject *self, PyObject *args)
989980
return input_obj;
990981
}
991982
/* Fix the size of the resulting string */
992-
if (inlen > 0 &&_PyString_Resize(&result, output-output_start))
993-
return NULL;
983+
if (inlen > 0)
984+
_PyString_Resize(&result, output - output_start);
994985
return result;
995986
}
996987

‎Modules/zlibmodule.c‎

Lines changed: 7 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -255,9 +255,8 @@ PyZlib_decompress(PyObject *self, PyObject *args)
255255
/* fall through */
256256
case(Z_OK):
257257
/* need more memory */
258-
if (_PyString_Resize(&result_str, r_strlen << 1) == -1) {
258+
if (_PyString_Resize(&result_str, r_strlen << 1) < 0) {
259259
inflateEnd(&zst);
260-
result_str = NULL;
261260
goto error;
262261
}
263262
zst.next_out = (unsigned char *)PyString_AS_STRING(result_str) \
@@ -414,10 +413,8 @@ PyZlib_objcompress(compobject *self, PyObject *args)
414413
/* while Z_OK and the output buffer is full, there might be more output,
415414
so extend the output buffer and try again */
416415
while (err == Z_OK && self->zst.avail_out == 0) {
417-
if (_PyString_Resize(&RetVal, length << 1) == -1) {
418-
RetVal = NULL;
416+
if (_PyString_Resize(&RetVal, length << 1) < 0)
419417
goto error;
420-
}
421418
self->zst.next_out = (unsigned char *)PyString_AS_STRING(RetVal) \
422419
+ length;
423420
self->zst.avail_out = length;
@@ -438,9 +435,7 @@ PyZlib_objcompress(compobject *self, PyObject *args)
438435
RetVal = NULL;
439436
goto error;
440437
}
441-
if (_PyString_Resize(&RetVal,
442-
self->zst.total_out - start_total_out) < 0)
443-
RetVal = NULL;
438+
_PyString_Resize(&RetVal, self->zst.total_out - start_total_out);
444439

445440
error:
446441
LEAVE_ZLIB
@@ -510,10 +505,8 @@ PyZlib_objdecompress(compobject *self, PyObject *args)
510505
if (max_length && length > max_length)
511506
length = max_length;
512507

513-
if (_PyString_Resize(&RetVal, length) == -1) {
514-
RetVal = NULL;
508+
if (_PyString_Resize(&RetVal, length) < 0)
515509
goto error;
516-
}
517510
self->zst.next_out = (unsigned char *)PyString_AS_STRING(RetVal) \
518511
+ old_length;
519512
self->zst.avail_out = length - old_length;
@@ -561,8 +554,7 @@ PyZlib_objdecompress(compobject *self, PyObject *args)
561554
goto error;
562555
}
563556

564-
if (_PyString_Resize(&RetVal, self->zst.total_out - start_total_out) < 0)
565-
RetVal = NULL;
557+
_PyString_Resize(&RetVal, self->zst.total_out - start_total_out);
566558

567559
error:
568560
LEAVE_ZLIB
@@ -612,10 +604,8 @@ PyZlib_flush(compobject *self, PyObject *args)
612604
/* while Z_OK and the output buffer is full, there might be more output,
613605
so extend the output buffer and try again */
614606
while (err == Z_OK && self->zst.avail_out == 0) {
615-
if (_PyString_Resize(&RetVal, length << 1) == -1) {
616-
RetVal = NULL;
607+
if (_PyString_Resize(&RetVal, length << 1) < 0)
617608
goto error;
618-
}
619609
self->zst.next_out = (unsigned char *)PyString_AS_STRING(RetVal) \
620610
+ length;
621611
self->zst.avail_out = length;
@@ -651,8 +641,7 @@ PyZlib_flush(compobject *self, PyObject *args)
651641
goto error;
652642
}
653643

654-
if (_PyString_Resize(&RetVal, self->zst.total_out - start_total_out) < 0)
655-
RetVal = NULL;
644+
_PyString_Resize(&RetVal, self->zst.total_out - start_total_out);
656645

657646
error:
658647
LEAVE_ZLIB

0 commit comments

Comments
 (0)