reverted the change of r56246 and keep the old behaviour of MB2WC/WC2MB; document it even more clearly and correct code using these functions incorrectly

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

interface/wx/strconv.h

@@ -255,6 +255,20 @@ public:
         out buffer, the @a outLen parameter should be one more to allow to
         properly @c NUL-terminate the string.
 
+        So to properly use this function you need to write:
+        @code
+            size_t lenConv = conv.MB2WC(NULL, in, 0);
+            if ( lenConv == wxCONV_FAILED )
+                ... handle error ...
+            // allocate 1 more character for the trailing NUL and also pass
+            // the size of the buffer to the function now
+            wchar_t *out = new wchar_t[lenConv + 1];
+            if ( conv.MB2WC(out, in, lenConv + 1) == wxCONV_FAILED )
+                ... handle error ...
+        @endcode
+        For this and other reasons, ToWChar() is strongly recommended as a
+        replacement.
+
         @param out
             The output buffer, may be @NULL if the caller is only
             interested in the length of the resulting string
@@ -277,7 +291,7 @@ public:
         called with a non-@NULL buffer, the @a n parameter should be the size
         of the buffer and so it should take into account the trailing @c NUL,
         which might take two or four bytes for some encodings (UTF-16 and
-        UTF-32) and not one.
+        UTF-32) and not one, i.e. GetMBNulLen().
     */
     virtual size_t WC2MB(char* buf, const wchar_t* psz, size_t n) const;
 };

src/common/strconv.cpp

@@ -355,9 +355,7 @@ wxMBConv::FromWChar(char *dst, size_t dstLen,
 
 size_t wxMBConv::MB2WC(wchar_t *outBuff, const char *inBuff, size_t outLen) const
 {
-    // add 1 to available buffer length because MB2WC() parameter counts the
+    size_t rc = ToWChar(outBuff, outLen, inBuff);
-    // number of non-NUL characters while ToWChar() counts everything
-    size_t rc = ToWChar(outBuff, outLen + 1, inBuff);
     if ( rc != wxCONV_FAILED )
     {
         // ToWChar() returns the buffer length, i.e. including the trailing
@@ -370,12 +368,10 @@ size_t wxMBConv::MB2WC(wchar_t *outBuff, const char *inBuff, size_t outLen) cons
 
 size_t wxMBConv::WC2MB(char *outBuff, const wchar_t *inBuff, size_t outLen) const
 {
-    const size_t nulLen = GetMBNulLen();
+    size_t rc = FromWChar(outBuff, outLen, inBuff);
-
-    size_t rc = FromWChar(outBuff, outLen + nulLen, inBuff);
     if ( rc != wxCONV_FAILED )
     {
-        rc -= nulLen;
+        rc -= GetMBNulLen();
     }
 
     return rc;
@@ -2351,13 +2347,12 @@ size_t wxMBConv_iconv::FromWChar(char *dst, size_t dstLen,
     if (ms_wcNeedsSwap)
     {
         // need to copy to temp buffer to switch endianness
-        // (doing WC_BSWAP twice on the original buffer won't help, as it
+        // (doing WC_BSWAP twice on the original buffer won't work, as it
         //  could be in read-only memory, or be accessed in some other thread)
-        tmpbuf = (wchar_t *)malloc(inbuflen + SIZEOF_WCHAR_T);
+        tmpbuf = (wchar_t *)malloc(inbuflen);
         for ( size_t i = 0; i < srcLen; i++ )
             tmpbuf[i] = WC_BSWAP(src[i]);
 
-        tmpbuf[srcLen] = L'\0';
         src = tmpbuf;
     }
 
@@ -2377,11 +2372,11 @@ size_t wxMBConv_iconv::FromWChar(char *dst, size_t dstLen,
         do
         {
             dst = tbuf;
-            outbuflen = 16;
+            outbuflen = WXSIZEOF(tbuf);
 
             cres = iconv(w2m, ICONV_CHAR_CAST(&inbuf), &inbuflen, &dst, &outbuflen);
 
-            res += 16 - outbuflen;
+            res += WXSIZEOF(tbuf) - outbuflen;
         }
         while ((cres == (size_t)-1) && (errno == E2BIG));
     }

src/msw/textctrl.cpp

@@ -990,15 +990,10 @@ wxTextCtrl::StreamIn(const wxString& value,
     if (len == wxCONV_FAILED)
         return false;
 
-#if wxUSE_WCHAR_T
+    wxWCharBuffer wchBuf(len); // allocates one extra character
-    wxWCharBuffer wchBuf(len);
     wchar_t *wpc = wchBuf.data();
-#else
-    wchar_t *wchBuf = (wchar_t *)malloc((len + 1)*sizeof(wchar_t));
-    wchar_t *wpc = wchBuf;
-#endif
 
-    conv.MB2WC(wpc, value.mb_str(), value.length());
+    conv.MB2WC(wpc, value.mb_str(), len + 1);
 #endif // wxUSE_UNICODE_MSLU
 
     // finally, stream it in the control

src/osx/carbon/textctrl.cpp

@@ -1371,8 +1371,10 @@ void wxMacMLTEControl::SetTXNData( const wxString& st, TXNOffset start, TXNOffse
 #else
     wxMBConvUTF16 converter ;
     ByteCount byteBufferLen = converter.WC2MB( NULL, st.wc_str(), 0 ) ;
-    UniChar *unibuf = (UniChar*)malloc( byteBufferLen ) ;
+    wxASSERT_MSG( byteBufferLen != wxCONV_FAILED,
-    converter.WC2MB( (char*)unibuf, st.wc_str(), byteBufferLen ) ;
+                  _T("Conversion to UTF-16 unexpectedly failed") );
+    UniChar *unibuf = (UniChar*)malloc( byteBufferLen + 2 ) ; // 2 for NUL in UTF-16
+    converter.WC2MB( (char*)unibuf, st.wc_str(), byteBufferLen + 2 ) ;
     TXNSetData( m_txn, kTXNUnicodeTextData, (void*)unibuf, byteBufferLen, start, end ) ;
     free( unibuf ) ;
 #endif

tests/mbconv/mbconvtest.cpp

@@ -815,41 +815,48 @@ void MBConvTestCase::BufSize()
     CPPUNIT_ASSERT( conv1251.IsOk() );
     const char *cp1251text =
         "\313\301\326\305\324\323\321 \325\304\301\336\316\331\315";
-    const size_t cp1251textLen = strlen(cp1251text);
 
-    const size_t lenW = conv1251.MB2WC(NULL, cp1251text, cp1251textLen);
+    const size_t lenW = conv1251.MB2WC(NULL, cp1251text, 0);
-    CPPUNIT_ASSERT( lenW != wxCONV_FAILED );
+    CPPUNIT_ASSERT_EQUAL( strlen(cp1251text), lenW );
-    wxWCharBuffer wbuf(lenW);
+    wxWCharBuffer wbuf(lenW + 1); // allocates lenW + 2 characters
+    wbuf.data()[lenW + 1] = L'!';
 
-    // lenW-1 is not enough
+    // lenW is not enough because it's the length and we need the size
     CPPUNIT_ASSERT_EQUAL(
-        wxCONV_FAILED, conv1251.MB2WC(wbuf.data(), cp1251text, lenW - 1) );
+        wxCONV_FAILED, conv1251.MB2WC(wbuf.data(), cp1251text, lenW) );
 
-    // lenW is just fine
+    // lenW+1 is just fine
-    CPPUNIT_ASSERT(
-        conv1251.MB2WC(wbuf.data(), cp1251text, lenW) != wxCONV_FAILED );
-
-    // of course, greater values work too
     CPPUNIT_ASSERT(
         conv1251.MB2WC(wbuf.data(), cp1251text, lenW + 1) != wxCONV_FAILED );
 
+    // of course, greater values work too
+    CPPUNIT_ASSERT(
+        conv1251.MB2WC(wbuf.data(), cp1251text, lenW + 2) != wxCONV_FAILED );
+
+    // but they shouldn't write more stuff to the buffer
+    CPPUNIT_ASSERT_EQUAL( L'!', wbuf[lenW + 1] );
+
 
     // test in the other direction too, using an encoding with multibyte NUL
-    wxCSConv convUTF16(_T("UTF-16"));
+    wxCSConv convUTF16(_T("UTF-16LE"));
     CPPUNIT_ASSERT( convUTF16.IsOk() );
     const wchar_t *utf16text = L"Hello";
-    const size_t utf16textLen = wcslen(utf16text);
 
-    const size_t lenMB = convUTF16.WC2MB(NULL, utf16text, utf16textLen);
+    const size_t lenMB = convUTF16.WC2MB(NULL, utf16text, 0);
-    CPPUNIT_ASSERT( lenMB != wxCONV_FAILED );
+    CPPUNIT_ASSERT_EQUAL( wcslen(utf16text)*2, lenMB );
-    wxCharBuffer buf(lenMB + 1); // it only adds 1 for NUL on its own, we need 2
+    wxCharBuffer buf(lenMB + 2); // it only adds 1 for NUL on its own, we need 2
+                                 // for NUL and an extra one for the guard byte
+    buf.data()[lenMB + 2] = '?';
 
     CPPUNIT_ASSERT_EQUAL(
-        wxCONV_FAILED, convUTF16.WC2MB(buf.data(), utf16text, lenMB - 1) );
+        wxCONV_FAILED, convUTF16.WC2MB(buf.data(), utf16text, lenMB) );
+    CPPUNIT_ASSERT_EQUAL(
+        wxCONV_FAILED, convUTF16.WC2MB(buf.data(), utf16text, lenMB + 1) );
     CPPUNIT_ASSERT(
-        convUTF16.WC2MB(buf.data(), utf16text, lenMB) != wxCONV_FAILED );
+        convUTF16.WC2MB(buf.data(), utf16text, lenMB + 2) != wxCONV_FAILED );
     CPPUNIT_ASSERT(
-        convUTF16.WC2MB(buf.data(), utf16text, lenMB + 1) != wxCONV_FAILED );
+        convUTF16.WC2MB(buf.data(), utf16text, lenMB + 3) != wxCONV_FAILED );
+    CPPUNIT_ASSERT_EQUAL( '?', buf[lenMB + 2] );
 }