[Scummvm-cvs-logs] SF.net SVN: scummvm:[54322] scummvm/trunk/common

[fingolfin at users.sourceforge.net]

Revision: 54322
          http://scummvm.svn.sourceforge.net/scummvm/?rev=54322&view=rev
Author:   fingolfin
Date:     2010-11-18 16:08:56 +0000 (Thu, 18 Nov 2010)

Log Message:
-----------
COMMON: Fix incorrect use of assert() macro

The assert() macro may be compiled to be empty. In that case, its
arguments are *NOT* evaluated. Hence, things like
  assert(doSomething())
must not be used whenever doSomething() has important side effects.

Also document BufferedWriteStream::flushBuffer() and explain why it
exists parallel to BufferedWriteStream::flush().

Modified Paths:
--------------
    scummvm/trunk/common/stream.cpp
    scummvm/trunk/common/stream.h

Modified: scummvm/trunk/common/stream.cpp
===================================================================
--- scummvm/trunk/common/stream.cpp	2010-11-18 15:42:52 UTC (rev 54321)
+++ scummvm/trunk/common/stream.cpp	2010-11-18 16:08:56 UTC (rev 54322)
@@ -331,7 +331,8 @@
 }
 
 BufferedWriteStream::~BufferedWriteStream() {
-	assert(flush());
+	const bool flushResult = flushBuffer();
+	assert(flushResult);
 
 	if (_disposeParentStream)
 		delete _parentStream;
@@ -345,20 +346,20 @@
 		memcpy(_buf + _pos, dataPtr, dataSize);
 		_pos += dataSize;
 	} else if (_bufSize >= dataSize) {	// check if we can flush the buffer and load the data
-		// flush the buffer
-		assert(flushBuffer());
+		const bool flushResult = flushBuffer();
+		assert(flushResult);
 		memcpy(_buf, dataPtr, dataSize);
 		_pos += dataSize;
 	} else	{	// too big for our buffer
-		// flush the buffer
-		assert(flushBuffer());
+		const bool flushResult = flushBuffer();
+		assert(flushResult);
 		return _parentStream->write(dataPtr, dataSize);
 	}
 	return dataSize;
 }
 
 bool BufferedWriteStream::flushBuffer() {
-	uint32 bytesToWrite = _pos;
+	const uint32 bytesToWrite = _pos;
 
 	if (bytesToWrite) {
 		_pos = 0;
@@ -368,10 +369,6 @@
 	return true;
 }
 
-bool BufferedWriteStream::flush() {
-	return flushBuffer();
-}
-
 bool MemoryWriteStreamDynamic::seek(int32 offs, int whence) {
 	// Pre-Condition
 	assert(_pos <= _size);

Modified: scummvm/trunk/common/stream.h
===================================================================
--- scummvm/trunk/common/stream.h	2010-11-18 15:42:52 UTC (rev 54321)
+++ scummvm/trunk/common/stream.h	2010-11-18 16:08:56 UTC (rev 54322)
@@ -534,15 +534,23 @@
 	DisposeAfterUse::Flag _disposeParentStream;
 	byte *_buf;
 	uint32 _pos;
-	uint32 _bufSize;
-	bool flushBuffer();						// write out the data in the buffer
+	const uint32 _bufSize;
 
+	/**
+	 * Write out the data in the buffer.
+	 *
+	 * @note This method is identical to flush() (which actually is
+	 * implemented by calling this method), except that it is not
+	 * virtual, hence there is less overhead calling it.
+	 */
+	bool flushBuffer();
+
 public:
 	BufferedWriteStream(WriteStream *parentStream, uint32 bufSize, DisposeAfterUse::Flag disposeParentStream = DisposeAfterUse::NO);
 	virtual ~BufferedWriteStream();
 
 	virtual uint32 write(const void *dataPtr, uint32 dataSize);
-	virtual bool flush();
+	virtual bool flush() { return flushBuffer(); }
 };
 
 /**


This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.