Re: svn commit: r25650 - in trunk/subversion: libsvn_subr tests/libsvn_subr

Hello

This commit breaks to work i18n on Windows XP SP2 Japanese Edition.

In my machine,
svn_subr__win32_xlate_open is called with setting 'frompage' to "CP932".
And get_page_id_from_name is called with setting 'page_name' to "CP932".

Then get_page_id_from_name passes by all if statements,
and mlang->lpVtbl->GetCharsetInfo fails with E_FAIL,
and finally returns APR_EINVAL.

But if I apply this patch, it works.
(I know the patch is not approprite. This is an example patch
to show why i18n doesn't work.)

Index: subversion/libsvn_subr/win32_xlate.c
===================================================================
--- subversion/libsvn_subr/win32_xlate.c (revision 25655)
+++ subversion/libsvn_subr/win32_xlate.c (working copy)
@@ -75,6 +75,11 @@
       *page_id_p = CP_UTF8;
       return APR_SUCCESS;
     }
+ else if (!strcmp(page_name, "CP932"))
+ {
+ *page_id_p = CP_ACP;
+ return APR_SUCCESS;
+ }

   hr = CoCreateInstance(&CLSID_CMultiLanguage, NULL, CLSCTX_INPROC_SERVER,
                         &IID_IMultiLanguage, (void **) &mlang);

Regards.

-- 
Masaru
2007/7/5, zhakov@tigris.org <zhakov@tigris.org>:
> Author: zhakov
> Date: Thu Jul  5 02:02:11 2007
> New Revision: 25650
>
> Log:
> Remove APR ICONV dependency on Windows and use native Windows
> character conversion API (MultiByteToWideChar/WideCharToMultiByte).
>
> * subversion/tests/libsvn_subr/utf-test.c
>   (test_utf_cstring_to_utf8_ex2): New test for conversion from
>    different codepages to utf8.
>   (test_utf_cstring_from_utf8_ex2): New test for conversion to
>    different codepages from utf8.
>   (test_funcs): Add two tests.
>
> * subversion/libsvn_subr/utf8.c
>   (get_xlate_handle_node): Use svn_subr__win32_xlate_open() on Windows.
>   (convert_to_stringbuf): Use svn_subr__win32_xlate_to_stringbuf() on Windows.
>
> * subversion/libsvn_subr/win32_xlate.c
> * subversion/libsvn_subr/win32_xlate.h
>   (win32_xlate_t): Structure for holding Windows xlate data.
>   (get_page_id_from_name): Helper for converting page name to page identifier.
>   (svn_subr__win32_xlate_open): Opens Windows xlate to convert one codepage to
>    another.
>   (svn_subr__win32_convert_to_string_buf): Converts string from one codepage to
>    another using Windows API.
>
> Added:
>    trunk/subversion/libsvn_subr/win32_xlate.c
>    trunk/subversion/libsvn_subr/win32_xlate.h
> Modified:
>    trunk/subversion/libsvn_subr/utf.c
>    trunk/subversion/tests/libsvn_subr/utf-test.c
>
> Modified: trunk/subversion/libsvn_subr/utf.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_subr/utf.c?pathrev=25650&r1=25649&r2=25650
> ==============================================================================
> --- trunk/subversion/libsvn_subr/utf.c  (original)
> +++ trunk/subversion/libsvn_subr/utf.c  Thu Jul  5 02:02:11 2007
> @@ -32,6 +32,7 @@
>  #include "svn_utf.h"
>  #include "utf_impl.h"
>  #include "svn_private_config.h"
> +#include "win32_xlate.h"
>
>
>
> @@ -252,10 +253,12 @@
>      pool = apr_hash_pool_get(xlate_handle_hash);
>
>    /* Try to create a handle. */
> -#ifndef AS400
> -  apr_err = apr_xlate_open(&handle, topage, frompage, pool);
> -#else
> +#if defined( WIN32)
> +  apr_err = svn_subr__win32_xlate_open(&handle, topage, frompage, pool);
> +#elif defined(AS400)
>    apr_err = apr_xlate_open(&handle, (int)topage, (int)frompage, pool);
> +#else
> +  apr_err = apr_xlate_open(&handle, topage, frompage, pool);
>  #endif
>
>    if (APR_STATUS_IS_EINVAL(apr_err) || APR_STATUS_IS_ENOTIMPL(apr_err))
> @@ -454,6 +457,12 @@
>                       svn_stringbuf_t **dest,
>                       apr_pool_t *pool)
>  {
> +#ifdef WIN32
> +  apr_status_t apr_err;
> +
> +  apr_err = svn_subr__win32_xlate_to_stringbuf(node->handle, src_data,
> +                                               src_length, dest, pool);
> +#else
>    apr_size_t buflen = src_length * 2;
>    apr_status_t apr_err;
>    apr_size_t srclen = src_length;
> @@ -507,6 +516,7 @@
>        (*dest)->len += ((buflen - (*dest)->len) - destlen);
>
>      } while (! apr_err && srclen);
> +#endif
>
>    /* If we exited the loop with an error, return the error. */
>    if (apr_err)
>
> Added: trunk/subversion/libsvn_subr/win32_xlate.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_subr/win32_xlate.c?pathrev=25650
> ==============================================================================
> --- (empty file)
> +++ trunk/subversion/libsvn_subr/win32_xlate.c  Thu Jul  5 02:02:11 2007
> @@ -0,0 +1,195 @@
> +/*
> + * win32_xlate.c : Windows xlate stuff.
> + *
> + * ====================================================================
> + * Copyright (c) 2007 CollabNet.  All rights reserved.
> + *
> + * This software is licensed as described in the file COPYING, which
> + * you should have received as part of this distribution.  The terms
> + * are also available at http://subversion.tigris.org/license-1.html.
> + * If newer versions of this license are posted there, you may use a
> + * newer version instead, at your option.
> + *
> + * This software consists of voluntary contributions made by many
> + * individuals.  For exact contribution history, see the revision
> + * history and logs, available at http://subversion.tigris.org/.
> + * ====================================================================
> + */
> +
> +#ifdef WIN32
> +
> +/* Define _WIN32_DCOM for CoInitializeEx(). */
> +#define _WIN32_DCOM
> +
> +#include <windows.h>
> +#include <mlang.h>
> +
> +#include <apr.h>
> +#include <apr_errno.h>
> +#include <apr_portable.h>
> +
> +#include "svn_pools.h"
> +#include "svn_string.h"
> +#include "svn_utf.h"
> +
> +#include "win32_xlate.h"
> +
> +typedef struct win32_xlate_t
> +{
> +  UINT from_page_id;
> +  UINT to_page_id;
> +} win32_xlate_t;
> +
> +static apr_status_t
> +get_page_id_from_name(UINT *page_id_p, const char *page_name, apr_pool_t *pool)
> +{
> +  IMultiLanguage * mlang = NULL;
> +  HRESULT hr;
> +  MIMECSETINFO page_info;
> +  WCHAR ucs2_page_name[128];
> +
> +  if (page_name == SVN_APR_DEFAULT_CHARSET)
> +    {
> +        *page_id_p = CP_ACP;
> +        return APR_SUCCESS;
> +    }
> +  else if (page_name == SVN_APR_LOCALE_CHARSET)
> +    {
> +      OSVERSIONINFO ver_info;
> +      ver_info.dwOSVersionInfoSize = sizeof(ver_info);
> +
> +      /* CP_THREAD_ACP supported only on Windows 2000 and later.*/
> +      if (GetVersionEx(&ver_info) && ver_info.dwMajorVersion >= 5
> +          && ver_info.dwPlatformId == VER_PLATFORM_WIN32_NT)
> +        {
> +          *page_id_p = CP_THREAD_ACP;
> +          return APR_SUCCESS;
> +        }
> +
> +      /* CP_THREAD_ACP isn't supported on current system, so get locale
> +         encoding name from APR. */
> +      page_name = apr_os_locale_encoding(pool);
> +    }
> +  else if (!strcmp(page_name, "UTF-8"))
> +    {
> +      *page_id_p = CP_UTF8;
> +      return APR_SUCCESS;
> +    }
> +
> +  hr = CoCreateInstance(&CLSID_CMultiLanguage, NULL, CLSCTX_INPROC_SERVER,
> +                        &IID_IMultiLanguage, (void **) &mlang);
> +
> +  if (FAILED(hr))
> +    return APR_EGENERAL;
> +
> +  /* Convert page name to wide string. */
> +  MultiByteToWideChar(CP_UTF8, 0, page_name, -1, ucs2_page_name,
> +                      sizeof(ucs2_page_name) / sizeof(ucs2_page_name[0]));
> +  memset(&page_info, 0, sizeof(page_info));
> +  hr = mlang->lpVtbl->GetCharsetInfo(mlang, ucs2_page_name, &page_info);
> +  if (FAILED(hr))
> +    {
> +      mlang->lpVtbl->Release(mlang);
> +      return APR_EINVAL;
> +    }
> +
> +  if (page_info.uiInternetEncoding)
> +    *page_id_p = page_info.uiInternetEncoding;
> +  else
> +    *page_id_p = page_info.uiCodePage;
> +
> +  mlang->lpVtbl->Release(mlang);
> +
> +  return APR_SUCCESS;
> +}
> +
> +apr_status_t
> +svn_subr__win32_xlate_open(win32_xlate_t **xlate_p, const char *topage,
> +                           const char *frompage, apr_pool_t *pool)
> +{
> +  UINT from_page_id, to_page_id;
> +  apr_status_t apr_err = APR_SUCCESS;
> +  win32_xlate_t *xlate;
> +  HRESULT hr;
> +
> +  /* First try to initialize for apartment-threaded object concurrency. */
> +  hr = CoInitializeEx(NULL, COINIT_APARTMENTTHREADED);
> +  if (hr == RPC_E_CHANGED_MODE)
> +    {
> +      /* COM already initalized for multi-threaded object concurrency. We are
> +         neutral to object concurrency so try to initalize it in the same way
> +         for us. */
> +      hr = CoInitializeEx(NULL, COINIT_MULTITHREADED);
> +    }
> +
> +  if (FAILED(hr))
> +    return APR_EGENERAL;
> +
> +  apr_err = get_page_id_from_name(&to_page_id, topage, pool);
> +  if (apr_err == APR_SUCCESS)
> +    apr_err = get_page_id_from_name(&from_page_id, frompage, pool);
> +
> +  if (apr_err == APR_SUCCESS)
> +    {
> +      xlate = apr_palloc(pool, sizeof(*xlate));
> +      xlate->from_page_id = from_page_id;
> +      xlate->to_page_id = to_page_id;
> +
> +      *xlate_p = xlate;
> +    }
> +
> +  CoUninitialize();
> +  return apr_err;
> +}
> +
> +apr_status_t
> +svn_subr__win32_xlate_to_stringbuf(win32_xlate_t *handle,
> +                                   const char *src_data,
> +                                   apr_size_t src_length,
> +                                   svn_stringbuf_t **dest,
> +                                   apr_pool_t *pool)
> +{
> +  WCHAR * wide_str;
> +  int retval, wide_size;
> +
> +  *dest = svn_stringbuf_create("", pool);
> +
> +  if (src_length == 0)
> +    return APR_SUCCESS;
> +
> +  retval = MultiByteToWideChar(handle->from_page_id, 0, src_data, src_length,
> +                               NULL, 0);
> +  if (retval == 0)
> +    return apr_get_os_error();
> +
> +  /* ### FIXME: Place for optimization. We can try allocate small strings
> +     temporary buffers on stack instead of heap.*/
> +  wide_size = retval;
> +  wide_str = apr_palloc(pool, wide_size * sizeof(WCHAR));
> +  retval = MultiByteToWideChar(handle->from_page_id, 0, src_data, src_length,
> +                               wide_str, wide_size);
> +
> +  if (retval == 0)
> +    return apr_get_os_error();
> +
> +  retval = WideCharToMultiByte(handle->to_page_id, 0, wide_str, wide_size,
> +                               NULL, 0, NULL, NULL);
> +
> +  if (retval == 0)
> +    return apr_get_os_error();
> +
> +  /* Ensure that buffer is enough to hold result string and termination
> +     character. */
> +  svn_stringbuf_ensure(*dest, retval + 1);
> +  (*dest)->len = retval;
> +
> +  retval = WideCharToMultiByte(handle->to_page_id, 0, wide_str, wide_size,
> +                               (*dest)->data, (*dest)->len, NULL, NULL);
> +  if (retval == 0)
> +    return apr_get_os_error();
> +
> +  (*dest)->len = retval;
> +  return APR_SUCCESS;
> +}
> +
> +#endif /* WIN32 */
>
> Added: trunk/subversion/libsvn_subr/win32_xlate.h
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_subr/win32_xlate.h?pathrev=25650
> ==============================================================================
> --- (empty file)
> +++ trunk/subversion/libsvn_subr/win32_xlate.h  Thu Jul  5 02:02:11 2007
> @@ -0,0 +1,47 @@
> +/*
> + * win32_xlate.h : Windows xlate stuff.
> + *
> + * ====================================================================
> + * Copyright (c) 2007 CollabNet.  All rights reserved.
> + *
> + * This software is licensed as described in the file COPYING, which
> + * you should have received as part of this distribution.  The terms
> + * are also available at http://subversion.tigris.org/license-1.html.
> + * If newer versions of this license are posted there, you may use a
> + * newer version instead, at your option.
> + *
> + * This software consists of voluntary contributions made by many
> + * individuals.  For exact contribution history, see the revision
> + * history and logs, available at http://subversion.tigris.org/.
> + * ====================================================================
> + */
> +
> +#ifndef SVN_LIBSVN_SUBR_WIN32_XLATE_H
> +#define SVN_LIBSVN_SUBR_WIN32_XLATE_H
> +
> +#ifdef WIN32
> +
> +/* Opaque translation buffer. */
> +typedef struct win32_xlate_t win32_xlate_t;
> +
> +/* Set *XLATE_P to a handle node for converting from FROMPAGE to TOPAGE.
> +   Returns APR_EINVAL or APR_ENOTIMPL, if a conversion isn't supported.
> +   If fail for any other reason, return the error.
> +
> +   Allocate *RET in POOL. */
> +apr_status_t svn_subr__win32_xlate_open(win32_xlate_t **xlate_p,
> +                                        const char *topage,
> +                                        const char *frompage,
> +                                        apr_pool_t *pool);
> +
> +/* Convert SRC_LENGTH bytes of SRC_DATA in NODE->handle, store the result
> +   in *DEST, which is allocated in POOL. */
> +apr_status_t svn_subr__win32_xlate_to_stringbuf(win32_xlate_t *handle,
> +                                                const char *src_data,
> +                                                apr_size_t src_length,
> +                                                svn_stringbuf_t **dest,
> +                                                apr_pool_t *pool);
> +
> +#endif /* WIN32 */
> +
> +#endif /* SVN_LIBSVN_SUBR_WIN32_XLATE_H */
>
> Modified: trunk/subversion/tests/libsvn_subr/utf-test.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/tests/libsvn_subr/utf-test.c?pathrev=25650&r1=25649&r2=25650
> ==============================================================================
> --- trunk/subversion/tests/libsvn_subr/utf-test.c       (original)
> +++ trunk/subversion/tests/libsvn_subr/utf-test.c       Thu Jul  5 02:02:11 2007
> @@ -18,6 +18,8 @@
>
>  #include "../svn_test.h"
>  #include "../../libsvn_subr/utf_impl.h"
> +#include "svn_utf.h"
> +#include "svn_pools.h"
>
>  /* Random number seed.  Yes, it's global, just pretend you can't see it. */
>  static apr_uint32_t diff_diff3_seed;
> @@ -222,6 +224,102 @@
>    return SVN_NO_ERROR;
>  }
>
> +/* Test conversion from different codepages to utf8. */
> +static svn_error_t *
> +test_utf_cstring_to_utf8_ex2(const char **msg,
> +                             svn_boolean_t msg_only,
> +                             svn_test_opts_t *opts,
> +                             apr_pool_t *pool)
> +{
> +  apr_size_t i;
> +  apr_pool_t *subpool = svn_pool_create(pool);
> +
> +  struct data {
> +      const char *string;
> +      const char *expected_result;
> +      const char *from_page;
> +  } tests[] = {
> +      {"ascii text\n", "ascii text\n", "unexistant-page"},
> +      {"Edelwei\xdf", "Edelwei\xc3\x9f", "ISO-8859-1"}
> +  };
> +
> +  *msg = "test svn_utf_cstring_to_utf8_ex2";
> +
> +  if (msg_only)
> +    return SVN_NO_ERROR;
> +
> +  for (i = 0; i < sizeof(tests) / sizeof(tests[0]); i++)
> +    {
> +      const char *dest;
> +
> +      svn_pool_clear(subpool);
> +
> +      SVN_ERR(svn_utf_cstring_to_utf8_ex2(&dest, tests[i].string,
> +                                          tests[i].from_page, pool));
> +
> +      if (strcmp(dest, tests[i].expected_result))
> +        {
> +          return svn_error_createf
> +            (SVN_ERR_TEST_FAILED, NULL,
> +             "svn_utf_cstring_to_utf8_ex2 ('%s', '%s') returned ('%s') "
> +             "instead of ('%s')",
> +             tests[i].string, tests[i].from_page,
> +             dest,
> +             tests[i].expected_result);
> +        }
> +    }
> +  svn_pool_destroy(subpool);
> +  return SVN_NO_ERROR;
> +}
> +
> +/* Test conversion to different codepages from utf8. */
> +static svn_error_t *
> +test_utf_cstring_from_utf8_ex2(const char **msg,
> +                               svn_boolean_t msg_only,
> +                               svn_test_opts_t *opts,
> +                               apr_pool_t *pool)
> +{
> +  apr_size_t i;
> +  apr_pool_t *subpool = svn_pool_create(pool);
> +
> +  struct data {
> +      const char *string;
> +      const char *expected_result;
> +      const char *to_page;
> +  } tests[] = {
> +      {"ascii text\n", "ascii text\n", "unexistant-page"},
> +      {"Edelwei\xc3\x9f", "Edelwei\xdf", "ISO-8859-1"}
> +  };
> +
> +  *msg = "test svn_utf_cstring_from_utf8_ex2";
> +
> +  if (msg_only)
> +    return SVN_NO_ERROR;
> +
> +  for (i = 0; i < sizeof(tests) / sizeof(tests[0]); i++)
> +    {
> +      const char *dest;
> +
> +      svn_pool_clear(subpool);
> +
> +      SVN_ERR(svn_utf_cstring_from_utf8_ex2(&dest, tests[i].string,
> +                                            tests[i].to_page, pool));
> +
> +      if (strcmp(dest, tests[i].expected_result))
> +        {
> +          return svn_error_createf
> +            (SVN_ERR_TEST_FAILED, NULL,
> +             "svn_utf_cstring_from_utf8_ex2 ('%s', '%s') returned ('%s') "
> +             "instead of ('%s')",
> +             tests[i].string, tests[i].to_page,
> +             dest,
> +             tests[i].expected_result);
> +        }
> +    }
> +  svn_pool_destroy(subpool);
> +  return SVN_NO_ERROR;
> +}
> +
>
>  /* The test table.  */
>
> @@ -230,5 +328,7 @@
>      SVN_TEST_NULL,
>      SVN_TEST_PASS(utf_validate),
>      SVN_TEST_PASS(utf_validate2),
> +    SVN_TEST_PASS(test_utf_cstring_to_utf8_ex2),
> +    SVN_TEST_PASS(test_utf_cstring_from_utf8_ex2),
>      SVN_TEST_NULL
>    };
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: svn-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: svn-help@subversion.tigris.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org