Merge branch 'mbconv-len-fix'

Return buffers of correct length from wxMBConv::cWC2MB() and cMB2WC().

include/wx/strconv.h

@@ -71,11 +71,19 @@ public:
                              const wchar_t *src, size_t srcLen = wxNO_LEN) const;
 
 
-    // Convenience functions for translating NUL-terminated strings: returns
+    // Convenience functions for translating NUL-terminated strings: return
-    // the buffer containing the converted string or NULL pointer if the
+    // the buffer containing the converted string or empty buffer if the
     // conversion failed.
-    const wxWCharBuffer cMB2WC(const char *in) const;
+    wxWCharBuffer cMB2WC(const char *in) const
-    const wxCharBuffer cWC2MB(const wchar_t *in) const;
+        { return DoConvertMB2WC(in, wxNO_LEN); }
+    wxCharBuffer cWC2MB(const wchar_t *in) const
+        { return DoConvertWC2MB(in, wxNO_LEN); }
+
+    wxWCharBuffer cMB2WC(const wxScopedCharBuffer& in) const
+        { return DoConvertMB2WC(in, in.length()); }
+    wxCharBuffer cWC2MB(const wxScopedWCharBuffer& in) const
+        { return DoConvertWC2MB(in, in.length()); }
+
 
     // Convenience functions for converting strings which may contain embedded
     // NULs and don't have to be NUL-terminated.
@@ -92,28 +100,22 @@ public:
     // number of characters converted, whether the last one of them was NUL or
     // not. But if inLen == wxNO_LEN then outLen doesn't account for the last
     // NUL even though it is present.
-    const wxWCharBuffer
+    wxWCharBuffer
         cMB2WC(const char *in, size_t inLen, size_t *outLen) const;
-    const wxCharBuffer
+    wxCharBuffer
         cWC2MB(const wchar_t *in, size_t inLen, size_t *outLen) const;
 
-    // And yet more convenience functions for converting the entire buffers:
-    // these are the simplest and least error-prone as you never need to bother
-    // with lengths/sizes directly.
-    const wxWCharBuffer cMB2WC(const wxScopedCharBuffer& in) const;
-    const wxCharBuffer cWC2MB(const wxScopedWCharBuffer& in) const;
-
     // convenience functions for converting MB or WC to/from wxWin default
 #if wxUSE_UNICODE
-    const wxWCharBuffer cMB2WX(const char *psz) const { return cMB2WC(psz); }
+    wxWCharBuffer cMB2WX(const char *psz) const { return cMB2WC(psz); }
-    const wxCharBuffer cWX2MB(const wchar_t *psz) const { return cWC2MB(psz); }
+    wxCharBuffer cWX2MB(const wchar_t *psz) const { return cWC2MB(psz); }
     const wchar_t* cWC2WX(const wchar_t *psz) const { return psz; }
     const wchar_t* cWX2WC(const wchar_t *psz) const { return psz; }
 #else // ANSI
     const char* cMB2WX(const char *psz) const { return psz; }
     const char* cWX2MB(const char *psz) const { return psz; }
-    const wxCharBuffer cWC2WX(const wchar_t *psz) const { return cWC2MB(psz); }
+    wxCharBuffer cWC2WX(const wchar_t *psz) const { return cWC2MB(psz); }
-    const wxWCharBuffer cWX2WC(const char *psz) const { return cMB2WC(psz); }
+    wxWCharBuffer cWX2WC(const char *psz) const { return cMB2WC(psz); }
 #endif // Unicode/ANSI
 
     // this function is used in the implementation of cMB2WC() to distinguish
@@ -162,7 +164,12 @@ public:
     virtual wxMBConv *Clone() const = 0;
 
     // virtual dtor for any base class
-    virtual ~wxMBConv();
+    virtual ~wxMBConv() { }
+
+private:
+    // Common part of single argument cWC2MB() and cMB2WC() overloads above.
+    wxCharBuffer DoConvertWC2MB(const wchar_t* pwz, size_t srcLen) const;
+    wxWCharBuffer DoConvertMB2WC(const char* psz, size_t srcLen) const;
 };
 
 // ----------------------------------------------------------------------------

interface/wx/strconv.h

@@ -190,7 +190,7 @@ public:
         invalid and @a outLen is set to 0 (and not @c wxCONV_FAILED for
         compatibility concerns).
     */
-    const wxWCharBuffer cMB2WC(const char* in,
+    wxWCharBuffer cMB2WC(const char* in,
                                size_t inLen,
                                size_t *outLen) const;
 
@@ -209,7 +209,7 @@ public:
 
         @since 2.9.1
      */
-    const wxWCharBuffer cMB2WC(const wxCharBuffer& buf) const;
+    wxWCharBuffer cMB2WC(const wxCharBuffer& buf) const;
 
     //@{
     /**
@@ -221,7 +221,7 @@ public:
         is defined as the correct return type (without const).
     */
     const char* cMB2WX(const char* psz) const;
-    const wxWCharBuffer cMB2WX(const char* psz) const;
+    wxWCharBuffer cMB2WX(const char* psz) const;
     //@}
 
     /**
@@ -234,7 +234,7 @@ public:
         Its parameters have the same meaning as the corresponding parameters of
         FromWChar(), please see the description of cMB2WC() for more details.
     */
-    const wxCharBuffer cWC2MB(const wchar_t* in,
+    wxCharBuffer cWC2MB(const wchar_t* in,
                               size_t inLen,
                               size_t *outLen) const;
 
@@ -253,7 +253,7 @@ public:
 
         @since 2.9.1
      */
-    const wxCharBuffer cWC2MB(const wxWCharBuffer& buf) const;
+    wxCharBuffer cWC2MB(const wxWCharBuffer& buf) const;
 
     //@{
     /**
@@ -264,7 +264,7 @@ public:
         defined as the correct return type (without const).
     */
     const wchar_t* cWC2WX(const wchar_t* psz) const;
-    const wxCharBuffer cWC2WX(const wchar_t* psz) const;
+    wxCharBuffer cWC2WX(const wchar_t* psz) const;
     //@}
 
     //@{
@@ -276,7 +276,7 @@ public:
         is defined as the correct return type (without const).
     */
     const char* cWX2MB(const wxChar* psz) const;
-    const wxCharBuffer cWX2MB(const wxChar* psz) const;
+    wxCharBuffer cWX2MB(const wxChar* psz) const;
     //@}
 
     //@{
@@ -288,7 +288,7 @@ public:
         defined as the correct return type (without const).
     */
     const wchar_t* cWX2WC(const wxChar* psz) const;
-    const wxWCharBuffer cWX2WC(const wxChar* psz) const;
+    wxWCharBuffer cWX2WC(const wxChar* psz) const;
     //@}
 
     /**

src/common/strconv.cpp

@@ -394,48 +394,7 @@ size_t wxMBConv::WC2MB(char *outBuff, const wchar_t *inBuff, size_t outLen) cons
     return rc;
 }
 
-wxMBConv::~wxMBConv()
+wxWCharBuffer
-{
-    // nothing to do here (necessary for Darwin linking probably)
-}
-
-const wxWCharBuffer wxMBConv::cMB2WC(const char *psz) const
-{
-    if ( psz )
-    {
-        // calculate the length of the buffer needed first
-        const size_t nLen = ToWChar(NULL, 0, psz);
-        if ( nLen != wxCONV_FAILED )
-        {
-            // now do the actual conversion
-            wxWCharBuffer buf(nLen - 1 /* +1 added implicitly */);
-
-            // +1 for the trailing NULL
-            if ( ToWChar(buf.data(), nLen, psz) != wxCONV_FAILED )
-                return buf;
-        }
-    }
-
-    return wxWCharBuffer();
-}
-
-const wxCharBuffer wxMBConv::cWC2MB(const wchar_t *pwz) const
-{
-    if ( pwz )
-    {
-        const size_t nLen = FromWChar(NULL, 0, pwz);
-        if ( nLen != wxCONV_FAILED )
-        {
-            wxCharBuffer buf(nLen - 1);
-            if ( FromWChar(buf.data(), nLen, pwz) != wxCONV_FAILED )
-                return buf;
-        }
-    }
-
-    return wxCharBuffer();
-}
-
-const wxWCharBuffer
 wxMBConv::cMB2WC(const char *inBuff, size_t inLen, size_t *outLen) const
 {
     const size_t dstLen = ToWChar(NULL, 0, inBuff, inLen);
@@ -470,7 +429,7 @@ wxMBConv::cMB2WC(const char *inBuff, size_t inLen, size_t *outLen) const
     return wxWCharBuffer();
 }
 
-const wxCharBuffer
+wxCharBuffer
 wxMBConv::cWC2MB(const wchar_t *inBuff, size_t inLen, size_t *outLen) const
 {
     size_t dstLen = FromWChar(NULL, 0, inBuff, inLen);
@@ -511,10 +470,13 @@ wxMBConv::cWC2MB(const wchar_t *inBuff, size_t inLen, size_t *outLen) const
     return wxCharBuffer();
 }
 
-const wxWCharBuffer wxMBConv::cMB2WC(const wxScopedCharBuffer& buf) const
+wxWCharBuffer wxMBConv::DoConvertMB2WC(const char* buf, size_t srcLen) const
 {
-    const size_t srcLen = buf.length();
+    // Notice that converting NULL pointer should work, i.e. return an empty
-    if ( srcLen )
+    // buffer instead of crashing, so we need to check both the length and the
+    // pointer because length is wxNO_LEN if it's a raw pointer and doesn't
+    // come from wxScopedCharBuffer.
+    if ( srcLen && buf )
     {
         const size_t dstLen = ToWChar(NULL, 0, buf, srcLen);
         if ( dstLen != wxCONV_FAILED )
@@ -522,17 +484,24 @@ const wxWCharBuffer wxMBConv::cMB2WC(const wxScopedCharBuffer& buf) const
             wxWCharBuffer wbuf(dstLen);
             wbuf.data()[dstLen] = L'\0';
             if ( ToWChar(wbuf.data(), dstLen, buf, srcLen) != wxCONV_FAILED )
+            {
+                // If the input string was NUL-terminated, we shouldn't include
+                // the length of the trailing NUL into the length of the return
+                // value.
+                if ( srcLen == wxNO_LEN )
+                    wbuf.shrink(dstLen - 1);
+
                 return wbuf;
+            }
         }
     }
 
-    return wxScopedWCharBuffer::CreateNonOwned(L"", 0);
+    return wxWCharBuffer();
 }
 
-const wxCharBuffer wxMBConv::cWC2MB(const wxScopedWCharBuffer& wbuf) const
+wxCharBuffer wxMBConv::DoConvertWC2MB(const wchar_t* wbuf, size_t srcLen) const
 {
-    const size_t srcLen = wbuf.length();
+    if ( srcLen && wbuf )
-    if ( srcLen )
     {
         const size_t dstLen = FromWChar(NULL, 0, wbuf, srcLen);
         if ( dstLen != wxCONV_FAILED )
@@ -540,11 +509,18 @@ const wxCharBuffer wxMBConv::cWC2MB(const wxScopedWCharBuffer& wbuf) const
             wxCharBuffer buf(dstLen);
             buf.data()[dstLen] = '\0';
             if ( FromWChar(buf.data(), dstLen, wbuf, srcLen) != wxCONV_FAILED )
+            {
+                // As above, in DoConvertMB2WC(), except that the length of the
+                // trailing NUL is variable in this case.
+                if ( srcLen == wxNO_LEN )
+                    buf.shrink(dstLen - GetMBNulLen());
+
                 return buf;
+            }
         }
     }
 
-    return wxScopedCharBuffer::CreateNonOwned("", 0);
+    return wxCharBuffer();
 }
 
 // ----------------------------------------------------------------------------

tests/mbconv/mbconvtest.cpp

@@ -874,6 +874,16 @@ void MBConvTestCase::BufSize()
     CPPUNIT_ASSERT(
         convUTF16.WC2MB(buf.data(), utf16text, lenMB + 3) != wxCONV_FAILED );
     CPPUNIT_ASSERT_EQUAL( '?', buf[lenMB + 2] );
+
+    // Test cWC2MB() too.
+    const wxCharBuffer buf2 = convUTF16.cWC2MB(utf16text);
+    CHECK( buf2.length() == lenMB );
+    CHECK( memcmp(buf, buf2, lenMB) == 0 );
+
+    const wxWCharBuffer utf16buf = wxWCharBuffer::CreateNonOwned(utf16text);
+    const wxCharBuffer buf3 = convUTF16.cWC2MB(utf16buf);
+    CHECK( buf3.length() == lenMB );
+    CHECK( memcmp(buf, buf3, lenMB) == 0 );
 }
 
 void MBConvTestCase::FromWCharTests()
@@ -1448,3 +1458,36 @@ void MBConvTestCase::UTF8(const char *charSequence,
 }
 
 #endif // HAVE_WCHAR_H
+
+TEST_CASE("wxMBConv::cWC2MB", "[mbconv][wc2mb]")
+{
+    wxMBConvUTF16 convUTF16;
+
+    CHECK( convUTF16.cWC2MB(L"").length() == 0 );
+    CHECK( convUTF16.cWC2MB(wxWCharBuffer()).length() == 0 );
+    CHECK( convUTF16.cWC2MB(L"Hi").length() == 4 );
+    CHECK( convUTF16.cWC2MB(wxWCharBuffer::CreateNonOwned(L"Hi")).length() == 4 );
+
+    CHECK( wxConvUTF7.cWC2MB(L"").length() == 0 );
+    CHECK( wxConvUTF7.cWC2MB(wxWCharBuffer()).length() == 0 );
+    CHECK( wxConvUTF7.cWC2MB(L"\xa3").length() == 5 );
+    // This test currently fails, the returned value is 3 because the
+    // conversion object doesn't return to its unshifted state -- which is
+    // probably a bug in wxMBConvUTF7.
+    // TODO: fix it there and reenable the test.
+    CHECK_NOFAIL( wxConvUTF7.cWC2MB(wxWCharBuffer::CreateNonOwned(L"\xa3")).length() == 5 );
+}
+
+TEST_CASE("wxMBConv::cMB2WC", "[mbconv][mb2wc]")
+{
+    wxMBConvUTF16 convUTF16;
+
+    CHECK( convUTF16.cMB2WC("\0").length() == 0 );
+    CHECK( convUTF16.cMB2WC(wxCharBuffer()).length() == 0 );
+    CHECK( convUTF16.cMB2WC("H\0i\0").length() == 2 );
+    CHECK( convUTF16.cMB2WC(wxCharBuffer::CreateNonOwned("H\0i\0", 4)).length() == 2 );
+
+    CHECK( wxConvUTF7.cMB2WC("").length() == 0 );
+    CHECK( wxConvUTF7.cMB2WC(wxCharBuffer()).length() == 0 );
+    CHECK( wxConvUTF7.cMB2WC("+AKM-").length() == 1 );
+}