Added some unit tests for Stream::readLine_NEW, and clarified that readLine_NEW is essentially fgets in disguise

svn-id: r34384

3 files changed, 53 insertions, 18 deletions

diff --git a/common/stream.cpp b/common/stream.cpp
index 4a411fadf5..d866fe0b5a 100644
--- a/common/stream.cpp
+++ b/common/stream.cpp
@@ -166,9 +166,12 @@ char *SeekableReadStream::readLine_NEW(char *buf, size_t bufSize) {
 		c = readByte();
 		
 		// If end-of-file occurs before any characters are read, return
-		// NULL and the buffer contents remain unchanged. If an error
-		/// occurs, return NULL and the buffer contents are indeterminate.
-		if (ioFailed() || (len == 0 && eos()))
+		// NULL and the buffer contents remain unchanged.
+		if (len == 0 && eos())
+			return 0;
+		
+		// If an error occurs, return NULL and the buffer contents are indeterminate.
+		if (ioFailed())
 			return 0;
 
 		// Check for CR or CR/LF
diff --git a/common/stream.h b/common/stream.h
index 81bb3dc91a..babb00e706 100644
--- a/common/stream.h
+++ b/common/stream.h
@@ -317,20 +317,7 @@ public:
 	void skip(uint32 offset) { seek(offset, SEEK_CUR); }
 
 	/**
-	 * Read one line of text from a CR or CR/LF terminated plain text file.
-	 * This method is a rough analog of the (f)gets function.
-	 *
-	 * @deprecated This method has a major flaw: There is no way to detect
-	 * whether a line exceeeds the length of the buffer, resulting in breakage
-	 * when overlong lines are encountered.
-	 * Use readLine_NEW() or readline() instead.
-	 *
-	 * @param buf	the buffer to store into
-	 * @param bufSize	the size of the buffer
-	 * @return a pointer to the read string, or NULL if an error occurred
-	 *
-	 * @note The line terminator (CR or CR/LF) is stripped and not inserted
-	 *       into the buffer.
+	 * DEPRECATED: Do not use this method! Instead use readLine_NEW() or readline().
 	 */
 	virtual char *readLine_OLD(char *buf, size_t bufSize);
 
@@ -350,6 +337,9 @@ public:
 	 * This method does not distinguish between end-of-file and error;
 	 * callers muse use ioFailed() or eos() to determine which occurred.
 	 *
+	 * @note This methods is closely modeled after the standard fgets()
+	 *       function from stdio.h.
+	 *
 	 * @param buf	the buffer to store into
 	 * @param bufSize	the size of the buffer
 	 * @return a pointer to the read string, or NULL if an error occurred
@@ -360,7 +350,7 @@ public:
 	/**
 	 * Reads a full line and returns it as a Common::String. Reading
 	 * stops when the end of a line is reached (CR, CR/LF or LF), and
-	 * at end-of-file or error. 
+	 * at end-of-file or error.
 	 *
 	 * Upon successful completion, return a string with the content
 	 * of the line, *without* the end of a line marker. This method
diff --git a/test/common/stream.h b/test/common/stream.h
new file mode 100644
index 0000000000..99e70d3a50
--- /dev/null
+++ b/test/common/stream.h
@@ -0,0 +1,42 @@
+#include <cxxtest/TestSuite.h>
+
+#include "common/stream.h"
+
+class ReadLineStreamTestSuite : public CxxTest::TestSuite {
+	public:
+	void test_readline(void) {
+		byte contents[] = { 'a', 'b', '\n', '\n', 'c', '\n' };
+		Common::MemoryReadStream ms(contents, sizeof(contents));
+		
+		char buffer[100];
+		
+		TS_ASSERT(0 != ms.readLine_NEW(buffer, sizeof(buffer)));
+		TS_ASSERT(0 == strcmp(buffer, "ab\n"));
+
+		TS_ASSERT(0 != ms.readLine_NEW(buffer, sizeof(buffer)));
+		TS_ASSERT(0 == strcmp(buffer, "\n"));
+
+		TS_ASSERT(0 != ms.readLine_NEW(buffer, sizeof(buffer)));
+		TS_ASSERT(0 == strcmp(buffer, "c\n"));
+
+		TS_ASSERT(ms.eos());
+	}
+
+	void test_readline2(void) {
+		byte contents[] = { 'a', 'b', '\n', '\n', 'c' };
+		Common::MemoryReadStream ms(contents, sizeof(contents));
+		
+		char buffer[100];
+		
+		TS_ASSERT(0 != ms.readLine_NEW(buffer, sizeof(buffer)));
+		TS_ASSERT(0 == strcmp(buffer, "ab\n"));
+
+		TS_ASSERT(0 != ms.readLine_NEW(buffer, sizeof(buffer)));
+		TS_ASSERT(0 == strcmp(buffer, "\n"));
+
+		TS_ASSERT(0 != ms.readLine_NEW(buffer, sizeof(buffer)));
+		TS_ASSERT(0 == strcmp(buffer, "c"));
+
+		TS_ASSERT(ms.eos());
+	}
+};