Date: prev next · Thread: first prev next last
2012 Archives by date, by thread · List index


In tools/qa/cppunit/test_stream.cxx:Test I added test_readline and after
poking around adding some edge-cases I see that Stream::ReadLine has the
odd quirk that it ignores embedded nulls in lines it reads, i.e. 
foo\0bar\n is read as "foobar", not "foo\0bar" or even "foo". That seems
kind of odd to me. Guessing that the very original code used String+=
I'd feel the original omission of a 0 from the read string was due to
the quick of String itself that it doesn't append trailing nulls and its
probably not deliberate ?

caolanm->erack:, from" erAck 26.02.01: Old behavior was no special
treatment of '\0'..." I presume you noticed this weirdness ages ago as
well in converting it from += to Append. What do you think, worth
syncing ReadLine behavior with std::getline behavior here ?

C.
diff --git a/tools/qa/cppunit/test_stream.cxx b/tools/qa/cppunit/test_stream.cxx
index 107d1c1..16293d0 100644
--- a/tools/qa/cppunit/test_stream.cxx
+++ b/tools/qa/cppunit/test_stream.cxx
@@ -264,16 +264,14 @@ namespace
         aMemStream.Seek(0);
         bRet = aMemStream.ReadLine(aFoo);
         CPPUNIT_ASSERT(bRet);
-        //This is the weird current behavior where an embedded null is read but
-        //discarded
-        CPPUNIT_ASSERT(aFoo.equalsL(RTL_CONSTASCII_STRINGPARAM("foobar"))); //<--diff A
+        CPPUNIT_ASSERT(aFoo.getLength() == 7 && aFoo[3] == 0);
         CPPUNIT_ASSERT(aMemStream.good());
 
         std::string sStr(RTL_CONSTASCII_STRINGPARAM(foo));
         std::istringstream iss(sStr, std::istringstream::in);
         std::getline(iss, sStr, '\n');
         //embedded null read as expected
-        CPPUNIT_ASSERT(sStr.size() == 7 && sStr[3] == 0);                   //<--diff A
+        CPPUNIT_ASSERT(sStr.size() == 7 && sStr[3] == 0);
         CPPUNIT_ASSERT(iss.good());
 
         bRet = aMemStream.ReadLine(aFoo);
@@ -299,12 +297,12 @@ namespace
         bRet = aMemStreamB.ReadLine(aFoo);
         CPPUNIT_ASSERT(bRet);
         CPPUNIT_ASSERT(aFoo.equalsL(RTL_CONSTASCII_STRINGPARAM("foo")));
-        CPPUNIT_ASSERT(!aMemStreamB.eof()); //<-- diff B
+        CPPUNIT_ASSERT(!aMemStreamB.eof()); //<-- diff A
 
         std::istringstream issB(bar, std::istringstream::in);
         std::getline(issB, sStr, '\n');
         CPPUNIT_ASSERT(sStr == "foo");
-        CPPUNIT_ASSERT(issB.eof());         //<-- diff B
+        CPPUNIT_ASSERT(issB.eof());         //<-- diff A
     }
 
     CPPUNIT_TEST_SUITE_REGISTRATION(Test);
diff --git a/tools/source/stream/stream.cxx b/tools/source/stream/stream.cxx
index 08559ad..7a9bf0d 100644
--- a/tools/source/stream/stream.cxx
+++ b/tools/source/stream/stream.cxx
@@ -643,7 +643,7 @@ sal_Bool SvStream::ReadLine( ByteString& rStr )
     sal_Char    c           = 0;
     sal_Size       nTotalLen   = 0;
 
-    rStr.Erase();
+    rtl::OStringBuffer aBuf;
     while( !bEnd && !GetError() )   // !!! nicht auf EOF testen,
                                     // !!! weil wir blockweise
                                     // !!! lesen
@@ -651,10 +651,11 @@ sal_Bool SvStream::ReadLine( ByteString& rStr )
         sal_uInt16 nLen = (sal_uInt16)Read( buf, sizeof(buf)-1 );
         if ( !nLen )
         {
-            if ( rStr.Len() == 0 )
+            if ( aBuf.getLength() == 0 )
             {
                 // der allererste Blockread hat fehlgeschlagen -> Abflug
                 bIsEof = sal_True;
+                rStr = rtl::OString();
                 return sal_False;
             }
             else
@@ -670,23 +671,15 @@ sal_Bool SvStream::ReadLine( ByteString& rStr )
                 bEnd = sal_True;
                 break;
             }
-            // erAck 26.02.01: Old behavior was no special treatment of '\0'
-            // character here, but a following rStr+=c did ignore it. Is this
-            // really intended? Or should a '\0' better terminate a line?
-            // The nOldFilePos stuff wasn't correct then anyways.
-            if ( c )
-            {
-                if ( n < j )
-                    buf[n] = c;
-                ++n;
-            }
+            if ( n < j )
+                buf[n] = c;
+            ++n;
         }
-        if ( n )
-            rStr.Append( buf, n );
+        aBuf.append(buf, n);
         nTotalLen += j;
     }
 
-    if ( !bEnd && !GetError() && rStr.Len() )
+    if ( !bEnd && !GetError() && aBuf.getLength() )
         bEnd = sal_True;
 
     nOldFilePos += nTotalLen;
@@ -706,6 +699,7 @@ sal_Bool SvStream::ReadLine( ByteString& rStr )
 
     if ( bEnd )
         bIsEof = sal_False;
+    rStr = aBuf.makeStringAndClear();
     return bEnd;
 }
 

Context


Privacy Policy | Impressum (Legal Info) | Copyright information: Unless otherwise specified, all text and images on this website are licensed under the Creative Commons Attribution-Share Alike 3.0 License. This does not include the source code of LibreOffice, which is licensed under the Mozilla Public License (MPLv2). "LibreOffice" and "The Document Foundation" are registered trademarks of their corresponding registered owners or are in actual use as trademarks in one or more countries. Their respective logos and icons are also subject to international copyright laws. Use thereof is explained in our trademark policy.