Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help)

Ivan Zhakov wrote:

> Sorry, I was busy and I have not time to report my research results.
>
> My patch almost complete and I need somebody's eyes to look and
> confirm that I'm on right way (or say that I'm on wrong way :)
>
> I used Windows MIME database to convert page name to Windows page
> identifier.
> And most important question is it acceptable to read Windows MIME
> database directly from registry instead of using MLang API [1]? I
> didn't use MLang API, because it requires
> CoInitialize()/CoUninitialize().

I think it would be better to use the API instead of reading the
registry. Because the registry is not documented and could change any time.
About CoInitialize(): we could call that in svn_utf_initialize() and
call CoUninitialize in an apr cleanup handler.

[snip]
+static apr_status_t
+win32_convert_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_FROM_OS_ERROR(GetLastError());
+
+ wide_size = retval;
+ wide_str = apr_palloc(pool, wide_size * sizeof(WCHAR));

I know this is the correct way to do this, but I don't like it. You
allocate memory here which is needed only for a few lines of code. That
memory is allocated in the apr pool, which we don't know when it gets
cleared again (maybe not until the app exits because it's the same pool
that holds the translated strings). That means that a lot of memory is
'wasted'.
For example, assume you do an
svn log -v url/to/kde/repository/root
and you'd have to keep that log information alive until your app exits.
With using an apr pool to reserve memory here that would mean that a
*lot* of memory is kept allocated but not used anymore.
That's why I used calloc() and free() in my patch - which isn't really
good either in terms of platform independence, but it won't consume a
lot of memory for nothing. Question is: do we really need apr pools here
for this Windows-specific code?
The correct way here may be to create a subpool and destroy that subpool
before the function exits. But that's an expensive operation.

In case you're wondering why someone would do such a thing (get the log
info for a whole repository and keep it until the app exits), the
revision graph in TSVN needs to do exactly that.

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.net
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org