[Python-checkins] CVS: python/dist/src/Modules zlibmodule.c,2.48,2.49

Jeremy Hylton jhylton@users.sourceforge.net
Tue, 16 Oct 2001 16:02:34 -0700


Update of /cvsroot/python/python/dist/src/Modules
In directory usw-pr-cvs1:/tmp/cvs-serv28392

Modified Files:
	zlibmodule.c 
Log Message:
Simplify and fix error handling for most cases.

Many functions used a local variable called return_error, which was
initialized to zero.  If an error occurred, it was set to true.  Most
of the code paths checked were only executed if return_error was
false.  goto is clearer.

The code also seemed to be written under the curious assumption that
calling Py_DECREF() on a local variable would assign the variable to
NULL.  As a result, more of the error-exit code paths returned an
object that had a reference count of zero instead of just returning
NULL.  Fixed the code to explicitly assign NULL after the DECREF.

A bit more reformatting, but not much.

XXX Need a much better test suite for zlib, since it the current tests
don't exercise any of this broken code.




Index: zlibmodule.c
===================================================================
RCS file: /cvsroot/python/python/dist/src/Modules/zlibmodule.c,v
retrieving revision 2.48
retrieving revision 2.49
diff -C2 -d -r2.48 -r2.49
*** zlibmodule.c	2001/10/16 21:59:35	2.48
--- zlibmodule.c	2001/10/16 23:02:32	2.49
***************
*** 137,141 ****
      int length, level=Z_DEFAULT_COMPRESSION, err;
      z_stream zst;
-     int return_error;
      PyObject * inputString;
    
--- 137,140 ----
***************
*** 150,156 ****
      zst.avail_out = length + length/1000 + 12 + 1;
  
!     output=(Byte*)malloc(zst.avail_out);
!     if (output==NULL) 
!     {
  	PyErr_SetString(PyExc_MemoryError,
  			"Can't allocate memory to compress data");
--- 149,154 ----
      zst.avail_out = length + length/1000 + 12 + 1;
  
!     output = (Byte*)malloc(zst.avail_out);
!     if (output == NULL) {
  	PyErr_SetString(PyExc_MemoryError,
  			"Can't allocate memory to compress data");
***************
*** 163,176 ****
      Py_INCREF(inputString);	/* increment so that we hold ref */
  
!     zst.zalloc=(alloc_func)NULL;
!     zst.zfree=(free_func)Z_NULL;
!     zst.next_out=(Byte *)output;
!     zst.next_in =(Byte *)input;
!     zst.avail_in=length;
!     err=deflateInit(&zst, level);
  
!     return_error = 0;
!     switch(err) 
!     {
      case(Z_OK):
  	break;
--- 161,172 ----
      Py_INCREF(inputString);	/* increment so that we hold ref */
  
!     zst.zalloc = (alloc_func)NULL;
!     zst.zfree = (free_func)Z_NULL;
!     zst.next_out = (Byte *)output;
!     zst.next_in = (Byte *)input;
!     zst.avail_in = length;
!     err = deflateInit(&zst, level);
  
!     switch(err) {
      case(Z_OK):
  	break;
***************
*** 178,215 ****
  	PyErr_SetString(PyExc_MemoryError,
  			"Out of memory while compressing data");
! 	return_error = 1;
! 	break;
      case(Z_STREAM_ERROR):
  	PyErr_SetString(ZlibError,
  			"Bad compression level");
! 	return_error = 1;
! 	break;
      default:
          deflateEnd(&zst);
  	zlib_error(zst, err, "while compressing data");
! 	return_error = 1;
      }
  
!     if (!return_error) {
! 	Py_BEGIN_ALLOW_THREADS;
!         err = deflate(&zst, Z_FINISH);
! 	Py_END_ALLOW_THREADS;
  
! 	if (err != Z_STREAM_END) {
! 	    zlib_error(zst, err, "while compressing data");
! 	    deflateEnd(&zst);
! 	    return_error = 1;
! 	}
!     
! 	if (!return_error) {
! 	    err=deflateEnd(&zst);
! 	    if (err == Z_OK)
! 		ReturnVal = PyString_FromStringAndSize((char *)output, 
! 						       zst.total_out);
! 	    else 
! 		zlib_error(zst, err, "while finishing compression");
! 	}
      }
  
      free(output);
      Py_DECREF(inputString);
--- 174,206 ----
  	PyErr_SetString(PyExc_MemoryError,
  			"Out of memory while compressing data");
! 	goto error;
      case(Z_STREAM_ERROR):
  	PyErr_SetString(ZlibError,
  			"Bad compression level");
! 	goto error;
      default:
          deflateEnd(&zst);
  	zlib_error(zst, err, "while compressing data");
! 	goto error;
      }
  
!     Py_BEGIN_ALLOW_THREADS;
!     err = deflate(&zst, Z_FINISH);
!     Py_END_ALLOW_THREADS;
  
!     if (err != Z_STREAM_END) {
! 	zlib_error(zst, err, "while compressing data");
! 	deflateEnd(&zst);
! 	goto error;
      }
+     
+     err=deflateEnd(&zst);
+     if (err == Z_OK)
+ 	ReturnVal = PyString_FromStringAndSize((char *)output, 
+ 					       zst.total_out);
+     else 
+ 	zlib_error(zst, err, "while finishing compression");
  
+  error:
      free(output);
      Py_DECREF(inputString);
***************
*** 232,236 ****
      int wsize=DEF_WBITS, r_strlen=DEFAULTALLOC;
      z_stream zst;
-     int return_error;
      PyObject * inputString;
  
--- 223,226 ----
***************
*** 261,265 ****
      err = inflateInit2(&zst, wsize);
  
-     return_error = 0;
      switch(err) {
      case(Z_OK):
--- 251,254 ----
***************
*** 268,282 ****
  	PyErr_SetString(PyExc_MemoryError,
  			"Out of memory while decompressing data");
! 	return_error = 1;
      default:
          inflateEnd(&zst);
  	zlib_error(zst, err, "while preparing to decompress data");
! 	return_error = 1;
      }
  
      do {
- 	if (return_error)
- 	    break;
- 
  	Py_BEGIN_ALLOW_THREADS
  	err=inflate(&zst, Z_FINISH);
--- 257,268 ----
  	PyErr_SetString(PyExc_MemoryError,
  			"Out of memory while decompressing data");
! 	goto error;
      default:
          inflateEnd(&zst);
  	zlib_error(zst, err, "while preparing to decompress data");
! 	goto error;
      }
  
      do {
  	Py_BEGIN_ALLOW_THREADS
  	err=inflate(&zst, Z_FINISH);
***************
*** 296,301 ****
  			     err);
  		inflateEnd(&zst);
! 		return_error = 1;
! 		break;
  	    }
  	    /* fall through */
--- 282,286 ----
  			     err);
  		inflateEnd(&zst);
! 		goto error;
  	    }
  	    /* fall through */
***************
*** 305,309 ****
  		inflateEnd(&zst);
  		result_str = NULL;
! 		return_error = 1;
  	    }
  	    zst.next_out = (unsigned char *)PyString_AsString(result_str) \
--- 290,294 ----
  		inflateEnd(&zst);
  		result_str = NULL;
! 		goto error;
  	    }
  	    zst.next_out = (unsigned char *)PyString_AsString(result_str) \
***************
*** 315,337 ****
  	    inflateEnd(&zst);
  	    zlib_error(zst, err, "while decompressing data");
! 	    return_error = 1;
  	}
      } while (err != Z_STREAM_END);
  
!     if (!return_error) {
! 	err = inflateEnd(&zst);
! 	if (err != Z_OK) {
! 	    zlib_error(zst, err, "while finishing data decompression");
! 	    return NULL;
! 	}
      }
  
!     if (!return_error)
! 	_PyString_Resize(&result_str, zst.total_out);
!     else
! 	Py_XDECREF(result_str);	/* sets result_str == NULL, if not already */
      Py_DECREF(inputString);
- 
      return result_str;
  }
  
--- 300,321 ----
  	    inflateEnd(&zst);
  	    zlib_error(zst, err, "while decompressing data");
! 	    goto error;
  	}
      } while (err != Z_STREAM_END);
  
!     err = inflateEnd(&zst);
!     if (err != Z_OK) {
! 	zlib_error(zst, err, "while finishing data decompression");
! 	return NULL;
      }
  
!     _PyString_Resize(&result_str, zst.total_out);
      Py_DECREF(inputString);
      return result_str;
+ 
+  error:
+     Py_DECREF(inputString);
+     Py_XDECREF(result_str);
+     return NULL;
  }
  
***************
*** 364,369 ****
      case(Z_STREAM_ERROR):
  	Py_DECREF(self);
! 	PyErr_SetString(PyExc_ValueError,
! 			"Invalid initialization option");
  	return NULL;
      default:
--- 348,352 ----
      case(Z_STREAM_ERROR):
  	Py_DECREF(self);
! 	PyErr_SetString(PyExc_ValueError, "Invalid initialization option");
  	return NULL;
      default:
***************
*** 383,387 ****
  
      self = newcompobject(&Decomptype);
!     if (self==NULL) 
  	return(NULL);
      self->zst.zalloc = (alloc_func)NULL;
--- 366,370 ----
  
      self = newcompobject(&Decomptype);
!     if (self == NULL) 
  	return(NULL);
      self->zst.zalloc = (alloc_func)NULL;
***************
*** 394,399 ****
      case(Z_STREAM_ERROR):
  	Py_DECREF(self);
! 	PyErr_SetString(PyExc_ValueError,
! 			"Invalid initialization option");
  	return NULL;
      case (Z_MEM_ERROR):
--- 377,381 ----
      case(Z_STREAM_ERROR):
  	Py_DECREF(self);
! 	PyErr_SetString(PyExc_ValueError, "Invalid initialization option");
  	return NULL;
      case (Z_MEM_ERROR):
***************
*** 452,460 ****
      Byte *input;
      unsigned long start_total_out;
!     int return_error;
!     PyObject * inputString;
    
      if (!PyArg_ParseTuple(args, "S:compress", &inputString))
  	return NULL;
      if (PyString_AsStringAndSize(inputString, (char**)&input, &inplen) == -1)
  	return NULL;
--- 434,442 ----
      Byte *input;
      unsigned long start_total_out;
!     PyObject *inputString;
    
      if (!PyArg_ParseTuple(args, "S:compress", &inputString))
  	return NULL;
+ 
      if (PyString_AsStringAndSize(inputString, (char**)&input, &inplen) == -1)
  	return NULL;
***************
*** 477,488 ****
      Py_END_ALLOW_THREADS
  
-     return_error = 0;
- 
      /* 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)  {
! 	    return_error = 1;
! 	    break;
  	}
  	self->zst.next_out = (unsigned char *)PyString_AsString(RetVal) \
--- 459,468 ----
      Py_END_ALLOW_THREADS
  
      /* 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;
! 	    goto error;
  	}
  	self->zst.next_out = (unsigned char *)PyString_AsString(RetVal) \
***************
*** 499,526 ****
         condition. 
      */  
- 
-     if (!return_error) {
- 	if (err != Z_OK && err != Z_BUF_ERROR) {
- 	    if (self->zst.msg == Z_NULL)
- 		PyErr_Format(ZlibError, "Error %i while compressing",
- 			     err); 
- 	    else
- 		PyErr_Format(ZlibError, "Error %i while compressing: %.200s",
- 			     err, self->zst.msg);  
  
! 	    return_error = 1;
! 	    Py_DECREF(RetVal);
! 	}
      }
! 
!     if (return_error)
! 	RetVal = NULL;		/* should have been handled by DECREF */
!     else
! 	_PyString_Resize(&RetVal, self->zst.total_out - start_total_out);
  
      Py_DECREF(inputString);
- 
      LEAVE_ZLIB
- 
      return RetVal;
  }
--- 479,496 ----
         condition. 
      */  
  
!     if (err != Z_OK && err != Z_BUF_ERROR) {
! 	zlib_error(self->zst, err, "while compressing");
! 	Py_DECREF(RetVal);
! 	RetVal = NULL;
! 	goto error;
      }
!     if (_PyString_Resize(&RetVal, 
! 			 self->zst.total_out - start_total_out) < 0)
! 	RetVal = NULL;
  
+  error:
      Py_DECREF(inputString);
      LEAVE_ZLIB
      return RetVal;
  }
***************
*** 545,549 ****
      Byte *input;
      unsigned long start_total_out;
-     int return_error;
      PyObject * inputString;
  
--- 515,518 ----
***************
*** 566,570 ****
  
      ENTER_ZLIB
-     return_error = 0;
  
      Py_INCREF(inputString);
--- 535,538 ----
***************
*** 597,602 ****
  
  	if (_PyString_Resize(&RetVal, length) == -1) {
! 	    return_error = 1;
! 	    break;
  	}
  	self->zst.next_out = (unsigned char *)PyString_AsString(RetVal) \
--- 565,570 ----
  
  	if (_PyString_Resize(&RetVal, length) == -1) {
! 	    RetVal = NULL;
! 	    goto error;
  	}
  	self->zst.next_out = (unsigned char *)PyString_AsString(RetVal) \
***************
*** 615,620 ****
  	self->unconsumed_tail = PyString_FromStringAndSize(self->zst.next_in, 
  							   self->zst.avail_in);
! 	if(!self->unconsumed_tail)
! 	    return_error = 1;
      }
  
--- 583,591 ----
  	self->unconsumed_tail = PyString_FromStringAndSize(self->zst.next_in, 
  							   self->zst.avail_in);
! 	if(!self->unconsumed_tail) {
! 	    Py_DECREF(RetVal);
! 	    RetVal = NULL;
! 	    goto error;
! 	}
      }
  
***************
*** 625,659 ****
         preserved.
      */
!     if (!return_error) {
! 	if (err == Z_STREAM_END) {
! 	    Py_XDECREF(self->unused_data);  /* Free original empty string */
! 	    self->unused_data = PyString_FromStringAndSize(
! 		(char *)self->zst.next_in, self->zst.avail_in);
! 	    if (self->unused_data == NULL) {
! 		Py_DECREF(RetVal);
! 		return_error = 1;
! 	    }
! 	    /* We will only get Z_BUF_ERROR if the output buffer was full 
! 	       but there wasn't more output when we tried again, so it is
! 	       not an error condition.
! 	    */
! 	} else if (err != Z_OK && err != Z_BUF_ERROR) {
! 	    if (self->zst.msg == Z_NULL)
! 		PyErr_Format(ZlibError, "Error %i while decompressing",
! 			     err); 
! 	    else
! 		PyErr_Format(ZlibError, "Error %i while decompressing: %.200s",
! 			     err, self->zst.msg);  
  	    Py_DECREF(RetVal);
! 	    return_error = 1;
  	}
      }
  
!     if (!return_error) {
! 	_PyString_Resize(&RetVal, self->zst.total_out - start_total_out);
!     }
!     else
! 	RetVal = NULL;		/* should be handled by DECREF */
  
      Py_DECREF(inputString);
  
--- 596,622 ----
         preserved.
      */
!     if (err == Z_STREAM_END) {
! 	Py_XDECREF(self->unused_data);  /* Free original empty string */
! 	self->unused_data = PyString_FromStringAndSize(
! 	    (char *)self->zst.next_in, self->zst.avail_in);
! 	if (self->unused_data == NULL) {
  	    Py_DECREF(RetVal);
! 	    goto error;
  	}
+ 	/* We will only get Z_BUF_ERROR if the output buffer was full 
+ 	   but there wasn't more output when we tried again, so it is
+ 	   not an error condition.
+ 	*/
+     } else if (err != Z_OK && err != Z_BUF_ERROR) {
+ 	zlib_error(self->zst, err, "while decompressing");
+ 	Py_DECREF(RetVal);
+ 	RetVal = NULL;
+ 	goto error;
      }
  
!     if (_PyString_Resize(&RetVal, self->zst.total_out - start_total_out) < 0)
! 	RetVal = NULL;
  
+  error:
      Py_DECREF(inputString);
  
***************
*** 678,682 ****
      int flushmode = Z_FINISH;
      unsigned long start_total_out;
-     int return_error;
  
      if (!PyArg_ParseTuple(args, "|i:flush", &flushmode))
--- 641,644 ----
***************
*** 703,714 ****
      Py_END_ALLOW_THREADS
  
-     return_error = 0;
- 
      /* 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)  {
! 	    return_error = 1;
! 	    break;
  	}
  	self->zst.next_out = (unsigned char *)PyString_AsString(RetVal) \
--- 665,674 ----
      Py_END_ALLOW_THREADS
  
      /* 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;
! 	    goto error;
  	}
  	self->zst.next_out = (unsigned char *)PyString_AsString(RetVal) \
***************
*** 725,769 ****
         various data structures. Note we should only get Z_STREAM_END when 
         flushmode is Z_FINISH, but checking both for safety*/
!     if (!return_error) {
! 	if (err == Z_STREAM_END && flushmode == Z_FINISH) {
! 	    err = deflateEnd(&(self->zst));
! 	    if (err != Z_OK) {
! 		if (self->zst.msg == Z_NULL)
! 		    PyErr_Format(ZlibError, "Error %i from deflateEnd()",
! 				 err); 
! 		else
! 		    PyErr_Format(ZlibError,
! 				 "Error %i from deflateEnd(): %.200s",
! 				 err, self->zst.msg);  
! 
! 		Py_DECREF(RetVal);
! 		return_error = 1;
! 	    } else
! 		self->is_initialised = 0;
! 
! 	    /* We will only get Z_BUF_ERROR if the output buffer was full 
! 	       but there wasn't more output when we tried again, so it is 
! 	       not an error condition.
! 	    */
! 	} else if (err!=Z_OK && err!=Z_BUF_ERROR) {
! 	    if (self->zst.msg == Z_NULL)
! 		PyErr_Format(ZlibError, "Error %i while flushing",
! 			     err); 
! 	    else
! 		PyErr_Format(ZlibError, "Error %i while flushing: %.200s",
! 			     err, self->zst.msg);  
  	    Py_DECREF(RetVal);
! 	    return_error = 1;
  	}
      }
- 
-     if (!return_error)
- 	_PyString_Resize(&RetVal, self->zst.total_out - start_total_out);
-     else
- 	RetVal = NULL;		/* should have been handled by DECREF */
      
!     LEAVE_ZLIB
  
! 	return RetVal;
  }
  
--- 685,716 ----
         various data structures. Note we should only get Z_STREAM_END when 
         flushmode is Z_FINISH, but checking both for safety*/
!     if (err == Z_STREAM_END && flushmode == Z_FINISH) {
! 	err = deflateEnd(&(self->zst));
! 	if (err != Z_OK) {
! 	    zlib_error(self->zst, err, "from deflateEnd()");
  	    Py_DECREF(RetVal);
! 	    RetVal = NULL;
! 	    goto error;
  	}
+ 	else
+ 	    self->is_initialised = 0;
+ 	
+ 	/* We will only get Z_BUF_ERROR if the output buffer was full 
+ 	   but there wasn't more output when we tried again, so it is 
+ 	   not an error condition.
+ 	*/
+     } else if (err!=Z_OK && err!=Z_BUF_ERROR) {
+ 	zlib_error(self->zst, err, "while flushing");
+ 	Py_DECREF(RetVal);
+ 	RetVal = NULL;
      }
      
!     if (_PyString_Resize(&RetVal, self->zst.total_out - start_total_out) < 0)
! 	RetVal = NULL;
  
!  error:    
!     LEAVE_ZLIB;
! 
!     return RetVal;
  }
  
***************
*** 781,785 ****
  {
      int err;
!     PyObject * retval;
    
      if (!PyArg_ParseTuple(args, ""))
--- 728,732 ----
  {
      int err;
!     PyObject * retval = NULL;
    
      if (!PyArg_ParseTuple(args, ""))
***************
*** 789,796 ****
  
      err = inflateEnd(&(self->zst));
!     if (err != Z_OK) {
  	zlib_error(self->zst, err, "from inflateEnd()");
! 	retval = NULL;
!     } else {
  	self->is_initialised = 0;
  	retval = PyString_FromStringAndSize(NULL, 0);
--- 736,742 ----
  
      err = inflateEnd(&(self->zst));
!     if (err != Z_OK)
  	zlib_error(self->zst, err, "from inflateEnd()");
!     else {
  	self->is_initialised = 0;
  	retval = PyString_FromStringAndSize(NULL, 0);