Login | Register
My pages Projects Community openCollabNet

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

subversion
Discussion topic

Back to topic list

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

Author steveking
Full name Stefan Küng
Date 2007-07-01 00:13:50 PDT
Message 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()/CoUni​nitialize().

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_st​ringbuf(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(Ge​tLastError());
+
+ 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

« Previous message in topic | 17 of 35 | Next message in topic »

Messages

Show all messages in topic

[PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) chemodax deleted 2007-06-30 14:45:41 PDT
     Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) djh D.J. Heap 2007-06-30 19:18:41 PDT
         Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) djh D.J. Heap 2007-06-30 19:43:51 PDT
             Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) chemodax deleted 2007-06-30 23:58:44 PDT
                 Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) djh D.J. Heap 2007-07-01 06:06:02 PDT
                     Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) Daniel Rall <dlr at collab dot net> Daniel Rall <dlr at collab dot net> 2007-07-03 09:58:00 PDT
                         Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) djh D.J. Heap 2007-07-04 07:48:13 PDT
             Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) chemodax deleted 2007-07-05 12:48:46 PDT
                 Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) djh D.J. Heap 2007-07-05 13:21:09 PDT
                     Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) Mark Phippard <markphip at gmail dot com> Mark Phippard <markphip at gmail dot com> 2007-07-05 13:23:59 PDT
                 Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) djh D.J. Heap 2007-07-07 08:31:40 PDT
                     Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) chemodax deleted 2007-07-07 15:32:02 PDT
                         Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) chemodax deleted 2007-07-07 15:47:49 PDT
                             Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) djh D.J. Heap 2007-07-07 18:47:34 PDT
                                 Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) chemodax deleted 2007-07-08 01:06:47 PDT
                         Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) djh D.J. Heap 2007-07-07 18:32:05 PDT
     Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) steveking Stefan Küng 2007-07-01 00:13:50 PDT
         Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) chemodax deleted 2007-07-01 08:31:56 PDT
             Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) steveking Stefan Küng 2007-07-01 08:52:52 PDT
                 Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) chemodax deleted 2007-07-02 04:07:15 PDT
                     Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) djh D.J. Heap 2007-07-02 20:49:17 PDT
                         Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) chemodax deleted 2007-07-05 02:11:42 PDT
                             Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) Mark Phippard <markphip at gmail dot com> Mark Phippard <markphip at gmail dot com> 2007-07-05 05:58:48 PDT
                                 Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) chemodax deleted 2007-07-05 06:07:05 PDT
                                 Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) steveking Stefan Küng 2007-07-05 06:10:53 PDT
Page: of 2 « Previous | Next »
Messages per page: