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.

diff --git a/Doc/api/concrete.tex b/Doc/api/concrete.tex
index 976292468d32d47b130d5a321600aa677f1c43ed..26a82d3d180ad8391e21e7f88a8747a304b772ff 100644 (file)
--- a/Doc/api/concrete.tex
+++ b/Doc/api/concrete.tex
@@ -586,7 +586,16 @@ parameter and are called with a non-string parameter.
 \begin{cfuncdesc}{int}{_PyString_Resize}{PyObject **string, int newsize}
   A way to resize a string object even though it is ``immutable''.
   Only use this to build up a brand new string object; don't use this
-  if the string may already be known in other parts of the code.
+  if the string may already be known in other parts of the code.  It
+  is an error to call this function if the refcount on the input string
+  object is not one.
+  Pass the address of an existing string object as an lvalue (it may
+  be written into), and the new size desired.  On success, \var{*string}
+  holds the resized string object and 0 is returned; the address in
+  \var{*string} may differ from its input value.  If the
+  reallocation fails, the original string object at \var{*string} is
+  deallocated, \var{*string} is set to \NULL{}, a memory exception is set,
+  and -1 is returned.
 \end{cfuncdesc}
 
 \begin{cfuncdesc}{PyObject*}{PyString_Format}{PyObject *format,

diff --git a/Modules/_ssl.c b/Modules/_ssl.c
index f5ab2b6e01534b8cca7709499df30f3b4a3260cd..1b05e40e6eaa57e1997e3cd87e387eda0e189624 100644 (file)
--- a/Modules/_ssl.c
+++ b/Modules/_ssl.c
@@ -331,8 +331,8 @@ static PyObject *PySSL_SSLread(PySSLObject *self, PyObject *args)
                Py_DECREF(buf);
                return PySSL_SetError(self, count);
        }
-       if (count != len && _PyString_Resize(&buf, count) < 0)
-               return NULL;
+       if (count != len)
+               _PyString_Resize(&buf, count);
        return buf;
 }
 

diff --git a/Modules/cPickle.c b/Modules/cPickle.c
index 2ffba48e0ebc086d41232f9cb16adc6a4a76da64..a6c5e9dc205fbd0e82fa7ecc0e0ad32025b63670 100644 (file)
--- a/Modules/cPickle.c
+++ b/Modules/cPickle.c
@@ -1252,14 +1252,8 @@ modified_EncodeRawUnicodeEscape(const Py_UNICODE *s, int size)
                        *p++ = (char) ch;
        }
        *p = '\0';
-       if (_PyString_Resize(&repr, p - q))
-               goto onError;
-
+       _PyString_Resize(&repr, p - q);
        return repr;
-
-  onError:
-       Py_DECREF(repr);
-       return NULL;
 }
 
 

diff --git a/Modules/cdmodule.c b/Modules/cdmodule.c
index b9772776b286e06168add8bb2357cea074f50478..25add3e1b61337e9d1fa4f13100e5b1a3237ce18 100644 (file)
--- a/Modules/cdmodule.c
+++ b/Modules/cdmodule.c
@@ -251,8 +251,7 @@ CD_readda(cdplayerobject *self, PyObject *args)
                return NULL;
        }
        if (n < numframes)
-               if (_PyString_Resize(&result, n * sizeof(CDFRAME)))
-                       return NULL;
+               _PyString_Resize(&result, n * sizeof(CDFRAME));
 
        return result;
 }

diff --git a/Modules/clmodule.c b/Modules/clmodule.c
index ef3d77a503033bfccdc9011c5c41998eded611a1..70e8f8e7d9a68b04285b90223e326da820c4dc1f 100644 (file)
--- a/Modules/clmodule.c
+++ b/Modules/clmodule.c
@@ -135,8 +135,7 @@ cl_CompressImage(PyObject *self, PyObject *args)
        }
 
        if (compressedBufferSize < frameBufferSize)
-               if (_PyString_Resize(&compressedBuffer, compressedBufferSize))
-                       return NULL;
+               _PyString_Resize(&compressedBuffer, compressedBufferSize);
 
        return compressedBuffer;
 }

diff --git a/Modules/linuxaudiodev.c b/Modules/linuxaudiodev.c
index fa2e441f6812e335c823e6e403bbb10aad14be19..7d4abc99cb77b5eb311051a7dff8d82e85f28b67 100644 (file)
--- a/Modules/linuxaudiodev.c
+++ b/Modules/linuxaudiodev.c
@@ -158,8 +158,7 @@ lad_read(lad_t *self, PyObject *args)
         return NULL;
     }
     self->x_icount += count;
-    if (_PyString_Resize(&rv, count) == -1)
-       return NULL;
+    _PyString_Resize(&rv, count);
     return rv;
 }
 

diff --git a/Modules/regexmodule.c b/Modules/regexmodule.c
index d449604cb8433811d6a20192e43e4c3e93c9988e..4c7035e501ecb8a439c41112b79371f718d9e6f7 100644 (file)
--- a/Modules/regexmodule.c
+++ b/Modules/regexmodule.c
@@ -516,11 +516,8 @@ symcomp(PyObject *pattern, PyObject *gdict)
                return NULL;
        }
        /* _PyString_Resize() decrements npattern on failure */
-       if (_PyString_Resize(&npattern, n - v) == 0)
-               return npattern;
-       else {
-               return NULL;
-       }
+       _PyString_Resize(&npattern, n - v);
+       return npattern;
 
 }
 

diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c
index 53952fcb9d495e63fd407c35ca3a074408dcd94e..21fcab0538d86e766ea9b2d86c6a88b939ae8a39 100644 (file)
--- a/Modules/socketmodule.c
+++ b/Modules/socketmodule.c
@@ -1402,8 +1402,8 @@ PySocketSock_recv(PySocketSockObject *s, PyObject *args)
                Py_DECREF(buf);
                return s->errorhandler();
        }
-       if (n != len && _PyString_Resize(&buf, n) < 0)
-               return NULL;
+       if (n != len)
+               _PyString_Resize(&buf, n);
        return buf;
 }
 

diff --git a/Modules/stropmodule.c b/Modules/stropmodule.c
index 0d5b75c5102bcbcf49950d0d88eca667e666527e..9a68b20edf800f8aff59a6fff5812e0021645a9c 100644 (file)
--- a/Modules/stropmodule.c
+++ b/Modules/stropmodule.c
@@ -215,10 +215,8 @@ strop_joinfields(PyObject *self, PyObject *args)
                        }
                        slen = PyString_GET_SIZE(item);
                        while (reslen + slen + seplen >= sz) {
-                               if (_PyString_Resize(&res, sz * 2)) {
-                                       Py_DECREF(res);
+                               if (_PyString_Resize(&res, sz * 2) < 0)
                                        return NULL;
-                               }
                                sz *= 2;
                                p = PyString_AsString(res) + reslen;
                        }
@@ -231,10 +229,7 @@ strop_joinfields(PyObject *self, PyObject *args)
                        p += slen;
                        reslen += slen;
                }
-               if (_PyString_Resize(&res, reslen)) {
-                       Py_DECREF(res);
-                       res = NULL;
-               }
+               _PyString_Resize(&res, reslen);
                return res;
        }
 
@@ -257,8 +252,7 @@ strop_joinfields(PyObject *self, PyObject *args)
                }
                slen = PyString_GET_SIZE(item);
                while (reslen + slen + seplen >= sz) {
-                       if (_PyString_Resize(&res, sz * 2)) {
-                               Py_DECREF(res);
+                       if (_PyString_Resize(&res, sz * 2) < 0) {
                                Py_DECREF(item);
                                return NULL;
                        }
@@ -275,10 +269,7 @@ strop_joinfields(PyObject *self, PyObject *args)
                reslen += slen;
                Py_DECREF(item);
        }
-       if (_PyString_Resize(&res, reslen)) {
-               Py_DECREF(res);
-               res = NULL;
-       }
+       _PyString_Resize(&res, reslen);
        return res;
 }
 
@@ -989,8 +980,8 @@ strop_translate(PyObject *self, PyObject *args)
                return input_obj;
        }
        /* Fix the size of the resulting string */
-       if (inlen > 0 &&_PyString_Resize(&result, output-output_start))
-               return NULL;
+       if (inlen > 0)
+               _PyString_Resize(&result, output - output_start);
        return result;
 }
 

diff --git a/Modules/zlibmodule.c b/Modules/zlibmodule.c
index a3891f71336a3e5b549f7137c88c0e3fef5d4b94..cfb71ae278e52e3f7020f77d75335fa6fc8bad68 100644 (file)
--- a/Modules/zlibmodule.c
+++ b/Modules/zlibmodule.c
@@ -255,9 +255,8 @@ PyZlib_decompress(PyObject *self, PyObject *args)
            /* fall through */
        case(Z_OK):
            /* need more memory */
-           if (_PyString_Resize(&result_str, r_strlen << 1) == -1) {
+           if (_PyString_Resize(&result_str, r_strlen << 1) < 0) {
                inflateEnd(&zst);
-               result_str = NULL;
                goto error;
            }
            zst.next_out = (unsigned char *)PyString_AS_STRING(result_str) \
@@ -414,10 +413,8 @@ PyZlib_objcompress(compobject *self, PyObject *args)
     /* while Z_OK and the output buffer is full, there might be more output,
        so extend the output buffer and try again */
     while (err == Z_OK && self->zst.avail_out == 0) {
-       if (_PyString_Resize(&RetVal, length << 1) == -1) {
-           RetVal = NULL;
+       if (_PyString_Resize(&RetVal, length << 1) < 0)
            goto error;
-       }
        self->zst.next_out = (unsigned char *)PyString_AS_STRING(RetVal) \
            + length;
        self->zst.avail_out = length;
@@ -438,9 +435,7 @@ PyZlib_objcompress(compobject *self, PyObject *args)
        RetVal = NULL;
        goto error;
     }
-    if (_PyString_Resize(&RetVal,
-                        self->zst.total_out - start_total_out) < 0)
-       RetVal = NULL;
+    _PyString_Resize(&RetVal, self->zst.total_out - start_total_out);
 
  error:
     LEAVE_ZLIB
@@ -510,10 +505,8 @@ PyZlib_objdecompress(compobject *self, PyObject *args)
        if (max_length && length > max_length)
            length = max_length;
 
-       if (_PyString_Resize(&RetVal, length) == -1) {
-           RetVal = NULL;
+       if (_PyString_Resize(&RetVal, length) < 0)
            goto error;
-       }
        self->zst.next_out = (unsigned char *)PyString_AS_STRING(RetVal) \
            + old_length;
        self->zst.avail_out = length - old_length;
@@ -561,8 +554,7 @@ PyZlib_objdecompress(compobject *self, PyObject *args)
        goto error;
     }
 
-    if (_PyString_Resize(&RetVal, self->zst.total_out - start_total_out) < 0)
-       RetVal = NULL;
+    _PyString_Resize(&RetVal, self->zst.total_out - start_total_out);
 
  error:
     LEAVE_ZLIB
@@ -612,10 +604,8 @@ PyZlib_flush(compobject *self, PyObject *args)
     /* while Z_OK and the output buffer is full, there might be more output,
        so extend the output buffer and try again */
     while (err == Z_OK && self->zst.avail_out == 0) {
-       if (_PyString_Resize(&RetVal, length << 1) == -1)  {
-           RetVal = NULL;
+       if (_PyString_Resize(&RetVal, length << 1) < 0)
            goto error;
-       }
        self->zst.next_out = (unsigned char *)PyString_AS_STRING(RetVal) \
            + length;
        self->zst.avail_out = length;
@@ -651,8 +641,7 @@ PyZlib_flush(compobject *self, PyObject *args)
        goto error;
     }
 
-    if (_PyString_Resize(&RetVal, self->zst.total_out - start_total_out) < 0)
-       RetVal = NULL;
+    _PyString_Resize(&RetVal, self->zst.total_out - start_total_out);
 
  error:
     LEAVE_ZLIB

diff --git a/Objects/fileobject.c b/Objects/fileobject.c
index 3dd10348b725e4c3f652b7e185be209952eeb049..029a270b02602fcbf4bd6ca2d8a945b041cb2511 100644 (file)
--- a/Objects/fileobject.c
+++ b/Objects/fileobject.c
@@ -1317,9 +1317,7 @@ file_readlines(PyFileObject *f, PyObject *args)
                        goto error;
        }
   cleanup:
-       if (big_buffer) {
-               Py_DECREF(big_buffer);
-       }
+       Py_XDECREF(big_buffer);
        return list;
 }
 

diff --git a/Objects/stringobject.c b/Objects/stringobject.c
index d3c9e4bdf75dcb049b840ea840ce8f4f1f061d9b..5bad04e55284c95ea573949be6d9e3a501c91e44 100644 (file)
--- a/Objects/stringobject.c
+++ b/Objects/stringobject.c
@@ -1869,8 +1869,8 @@ string_translate(PyStringObject *self, PyObject *args)
                return input_obj;
        }
        /* Fix the size of the resulting string */
-       if (inlen > 0 &&_PyString_Resize(&result, output-output_start))
-               return NULL;
+       if (inlen > 0)
+               _PyString_Resize(&result, output - output_start);
        return result;
 }
 
@@ -2927,7 +2927,14 @@ PyString_ConcatAndDel(register PyObject **pv, register PyObject *w)
    is only one module referencing the object.  You can also think of it
    as creating a new string object and destroying the old one, only
    more efficiently.  In any case, don't use this if the string may
-   already be known to some other part of the code... */
+   already be known to some other part of the code...
+   Note that if there's not enough memory to resize the string, the original
+   string object at *pv is deallocated, *pv is set to NULL, an "out of
+   memory" exception is set, and -1 is returned.  Else (on success) 0 is
+   returned, and the value in *pv may or may not be the same as on input.
+   As always, an extra byte is allocated for a trailing \0 byte (newsize
+   does *not* include that), and a trailing \0 byte is stored.
+*/
 
 int
 _PyString_Resize(PyObject **pv, int newsize)

diff --git a/Objects/unicodeobject.c b/Objects/unicodeobject.c
index c4c8d8d64098e84db5a6e57976238a0beca43908..9a3e4568660fc112c1252c7004674ab6b523a240 100644 (file)
--- a/Objects/unicodeobject.c
+++ b/Objects/unicodeobject.c
@@ -927,10 +927,7 @@ PyObject *PyUnicode_EncodeUTF7(const Py_UNICODE *s,
         *out++ = '-';
     }
 
-    if (_PyString_Resize(&v, out - start)) {
-        Py_DECREF(v);
-        return NULL;
-    }
+    _PyString_Resize(&v, out - start);
     return v;
 }
 
@@ -1764,7 +1761,7 @@ PyObject *unicodeescape_string(const Py_UNICODE *s,
            /* Resize the string if necessary */
            if (offset + 12 > PyString_GET_SIZE(repr)) {
                if (_PyString_Resize(&repr, PyString_GET_SIZE(repr) + 100))
-                   goto onError;
+                   return NULL;
                p = PyString_AS_STRING(repr) + offset;
            }
 
@@ -1847,14 +1844,8 @@ PyObject *unicodeescape_string(const Py_UNICODE *s,
         *p++ = PyString_AS_STRING(repr)[1];
 
     *p = '\0';
-    if (_PyString_Resize(&repr, p - PyString_AS_STRING(repr)))
-       goto onError;
-
+    _PyString_Resize(&repr, p - PyString_AS_STRING(repr));
     return repr;
-
- onError:
-    Py_DECREF(repr);
-    return NULL;
 }
 
 PyObject *PyUnicode_EncodeUnicodeEscape(const Py_UNICODE *s,
@@ -1985,14 +1976,8 @@ PyObject *PyUnicode_EncodeRawUnicodeEscape(const Py_UNICODE *s,
             *p++ = (char) ch;
     }
     *p = '\0';
-    if (_PyString_Resize(&repr, p - q))
-       goto onError;
-
+    _PyString_Resize(&repr, p - q);
     return repr;
-
- onError:
-    Py_DECREF(repr);
-    return NULL;
 }
 
 PyObject *PyUnicode_AsRawUnicodeEscapeString(PyObject *unicode)
@@ -2092,8 +2077,7 @@ PyObject *PyUnicode_EncodeLatin1(const Py_UNICODE *p,
     }
     /* Resize if error handling skipped some characters */
     if (s - start < PyString_GET_SIZE(repr))
-       if (_PyString_Resize(&repr, s - start))
-           goto onError;
+       _PyString_Resize(&repr, s - start);
     return repr;
 
  onError:
@@ -2240,8 +2224,7 @@ PyObject *PyUnicode_EncodeASCII(const Py_UNICODE *p,
     }
     /* Resize if error handling skipped some characters */
     if (s - start < PyString_GET_SIZE(repr))
-       if (_PyString_Resize(&repr, s - start))
-           goto onError;
+       _PyString_Resize(&repr, s - start);
     return repr;
 
  onError:
@@ -2588,12 +2571,11 @@ PyObject *PyUnicode_EncodeCharmap(const Py_UNICODE *p,
        Py_DECREF(x);
     }
     if (s - PyString_AS_STRING(v) < PyString_GET_SIZE(v))
-       if (_PyString_Resize(&v, (int)(s - PyString_AS_STRING(v))))
-           goto onError;
+       _PyString_Resize(&v, (int)(s - PyString_AS_STRING(v)));
     return v;
 
  onError:
-    Py_DECREF(v);
+    Py_XDECREF(v);
     return NULL;
 }
 

diff --git a/Python/bltinmodule.c b/Python/bltinmodule.c
index 06d86167aa72317e13bd346577ce55e189c5ac04..aa65ebca410d628213d29e1956e8d1d073711aeb 100644 (file)
--- a/Python/bltinmodule.c
+++ b/Python/bltinmodule.c
@@ -1993,8 +1993,8 @@ filterstring(PyObject *func, PyObject *strobj)
                Py_DECREF(item);
        }
 
-       if (j < len && _PyString_Resize(&result, j) < 0)
-               return NULL;
+       if (j < len)
+               _PyString_Resize(&result, j);
 
        return result;