Fix another off-by-1 bug in wxMBConv::ToWChar().

When converting a fixed number of characters we need to take any NULs inside
the buffer being converted into account for our return value -- but this
wasn't done and converting 2 characters 'x' and '\0' returned only 1, even if
the length 2 was explicitly specified.

Fix this bug and add a unit test checking for it.

git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@62141 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775

diff --git a/src/common/strconv.cpp b/src/common/strconv.cpp
index c0d9daf06b2e59b8d78a43f0a16012e1f4b51df4..7ad34c9ccf44a89ee1cbddc1af9e8448f007b84d 100644 (file)
--- a/src/common/strconv.cpp
+++ b/src/common/strconv.cpp
@@ -216,7 +216,7 @@ wxMBConv::ToWChar(wchar_t *dst, size_t dstLen,
     // all the complication come from the fact that this function, for
     // historical reasons, must behave in 2 subtly different ways when it's
     // called with a fixed number of characters and when it's called for the
-    // entire NUL-terminated string: in the former case (srcEnd == NULL) we
+    // entire NUL-terminated string: in the former case (srcEnd != NULL) we
     // must count all characters we convert, NUL or not; but in the latter we
     // do not count the trailing NUL -- but still count all the NULs inside the
     // string
@@ -258,11 +258,13 @@ wxMBConv::ToWChar(wchar_t *dst, size_t dstLen,
         if ( !srcEnd )
         {
             // we convert just one chunk in this case as this is the entire
-            // string anyhow
+            // string anyhow (and we don't count the trailing NUL in this case)
             break;
         }
 
-        // advance the input pointer past the end of this chunk
+        // advance the input pointer past the end of this chunk: notice that we
+        // will always stop before srcEnd because we know that the chunk is
+        // always properly NUL-terminated
         while ( NotAllNULs(src, nulLen) )
         {
             // notice that we must skip over multiple bytes here as we suppose
@@ -272,23 +274,20 @@ wxMBConv::ToWChar(wchar_t *dst, size_t dstLen,
             src += nulLen;
         }
 
-        src += nulLen; // skipping over its terminator as well
+        // if the buffer ends before this NUL, we shouldn't count it in our
+        // output so skip the code below
+        if ( src == srcEnd )
+            break;
+
+        // do count this terminator as it's inside the buffer we convert
+        dstWritten++;
+        if ( dst )
+            dst++;
+
+        src += nulLen; // skip the terminator itself
 
-        // note that ">=" (and not just "==") is needed here as the terminator
-        // we skipped just above could be inside or just after the buffer
-        // delimited by srcEnd
         if ( src >= srcEnd )
             break;
-
-        // if we got here then this wasn't the last chunk in this string and
-        // hence we must count an extra char for L'\0' even when converting a
-        // fixed number of characters
-        if ( srcEnd )
-        {
-            dstWritten++;
-            if ( dst )
-                dst++;
-        }
     }
 
     return dstWritten;
@@ -333,7 +332,7 @@ wxMBConv::FromWChar(char *dst, size_t dstLen,
             return wxCONV_FAILED;
 
         dstWritten += lenChunk;
-        if ( src+lenChunk < srcEnd || isNulTerminated )
+        if ( src + lenChunk < srcEnd || isNulTerminated )
             dstWritten += lenNul;
 
         if ( dst )
@@ -345,7 +344,7 @@ wxMBConv::FromWChar(char *dst, size_t dstLen,
                 return wxCONV_FAILED;
 
             dst += lenChunk;
-            if ( src+lenChunk < srcEnd || isNulTerminated )
+            if ( src + lenChunk < srcEnd || isNulTerminated )
                 dst += lenNul;
         }
     }

diff --git a/tests/strings/unicode.cpp b/tests/strings/unicode.cpp
index 18bd0f286f7ea0dfeb4d42a62c20ecd40a41cbb4..1a4e4c412bcc9e57be322f5a875902d053484466 100644 (file)
--- a/tests/strings/unicode.cpp
+++ b/tests/strings/unicode.cpp
@@ -139,7 +139,7 @@ private:
     CPPUNIT_TEST_SUITE( UnicodeTestCase );
         CPPUNIT_TEST( ToFromAscii );
         CPPUNIT_TEST( ConstructorsWithConversion );
-        CPPUNIT_TEST( ConversionEmpty );
+        CPPUNIT_TEST( ConversionFixed );
         CPPUNIT_TEST( ConversionWithNULs );
         CPPUNIT_TEST( ConversionUTF7 );
         CPPUNIT_TEST( ConversionUTF8 );
@@ -153,7 +153,7 @@ private:
 
     void ToFromAscii();
     void ConstructorsWithConversion();
-    void ConversionEmpty();
+    void ConversionFixed();
     void ConversionWithNULs();
     void ConversionUTF7();
     void ConversionUTF8();
@@ -238,7 +238,7 @@ void UnicodeTestCase::ConstructorsWithConversion()
     CPPUNIT_ASSERT( s5 != "SomethingElse" );
 }
 
-void UnicodeTestCase::ConversionEmpty()
+void UnicodeTestCase::ConversionFixed()
 {
     size_t len;
 
@@ -249,6 +249,15 @@ void UnicodeTestCase::ConversionEmpty()
 #endif // wxUSE_UNICODE/!wxUSE_UNICODE
 
     CPPUNIT_ASSERT_EQUAL( 0, len );
+
+#if wxUSE_UNICODE
+    // check that when we convert a fixed number of characters we obtain the
+    // expected return value
+    CPPUNIT_ASSERT_EQUAL( 0, wxConvLibc.ToWChar(NULL, 0, "", 0) );
+    CPPUNIT_ASSERT_EQUAL( 1, wxConvLibc.ToWChar(NULL, 0, "x", 1) );
+    CPPUNIT_ASSERT_EQUAL( 2, wxConvLibc.ToWChar(NULL, 0, "x", 2) );
+    CPPUNIT_ASSERT_EQUAL( 2, wxConvLibc.ToWChar(NULL, 0, "xy", 2) );
+#endif // wxUSE_UNICODE
 }
 
 void UnicodeTestCase::ConversionWithNULs()