apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "William A. Rowe, Jr." <wr...@rowe-clan.net>
Subject Re: apr-iconv 1.0
Date Wed, 30 Mar 2005 17:19:20 GMT
Steve,

  This is exactly my desired solution.  If you would like to
submit your patch to the list (submissions by reference aren't
accepted without extra hassles) I would very much like to adopt 
it.  (Especially as opposed to reinventing it.)

  My proposal of LD_LIBRARY_PATH corresponds to the fact that
Unix doesn't have the GetModuleFileName concept to figure out
where the absolute path of the libapriconv-1.so module lies.
Since we have to make exceptions for Win32, and HP/UX, I see
no reason not to use the native solution to Win32.  

  Incidentally, do we need to search DYLD_LIBRARY_PATH or 
LD_LIBRARY_PATH on OSX?

Bill

At 09:49 AM 3/30/2005, SteveKing wrote:
>Branko Čibej wrote:
>>William A. Rowe, Jr. wrote:
>>
>>>At 12:36 AM 3/30/2005, Curt Arnold wrote:
>>> 
>>>
>>>>However, it would be good if we could get some comment from a Subversion developer.
>>>>  
>>>
>>>Agreed, and Branko's watching this thread.
>>> 
>>Yup.
>>It might also be a good thing to ping Steve King of TortoiseSVN fame; TSVN also ships
a hacked version of apr-iconv, I believe the hack is to ignore APR_ICONV_PATH. The issue IIRC
is collision between memory allocators in different runtime librararies on Windows; but anyway,
Steve should be able to explain.
>
>The reasons why TSVN ships a patched version of apr-iconv:
>
>- the *.so files are actually dll's which require a specific C-runtime. This isn't a big
problem if APR_ICONV_PATH points to a iconv compiled with VC6. But if it points to an iconv
compiled with >VC7, then programs not using the new c-runtime (> version 7) won't run
anymore. A 'solution' to that problem would be to install the runtime in e.g. the system dir
on windows, but even MS tells to _not_ do that.
>
>- every program which installs its own iconv lib might change the APR_ICONV_PATH variable
(believe me: most programs _don't_ check if it's already set) and overwrite this with maybe
an older version of the lib!
>
>- looking up the env variable isn't very fast, i.e. it takes more time to load the *.so
modules than other methods.
>
>The changes I made to apr-iconv for TSVN:
>
>- add a DllMain() to iconv_module which caches the path to the *.so modules (i.e. the
path is determined only once when the iconv dll is loaded, not for every *.so module) - this
is a _very_ big speed improvement for Subversion which makes intensive use of *.so modules.
>
>- the path is determined like this now:
>  (in the following description, THISPATH refers to the path where the libapriconv.dll
is located, fetched via GetModuleFileName() API)
>  1. path exists THISPATH\iconv
>  2. path exists ..\THISPATH\iconv
>  3. use APR_ICONV_PATH
>  by doing this, it's assured that a program like TSVN always uses the iconv lib it has
installed itself. Only if it doesn't install the iconv lib itself or it can't be found in
1) or 2), then the APR_ICONV_PATH env variable is used to find the *.so modules.
>
>You can find the patched iconv_module.c file here:
>https://svn.collab.net/repos/tortoisesvn/trunk/ext/apr-iconv_patch/lib/iconv_module.c
>
>Ever since TSVN ships with this patched iconv version, we haven't had any more problems
with other Subversion clients interfering or missing c-runtime dll's or memory leaks due to
incompatible c-runtimes (according to MS, the version 6 and 7 of the c-runtime are not compatible
and use different memory allocators which leads to memory leaks and even heap corruptions
if a program uses the wrong version).
>
>So I'm not sure what you guys have planned to change in apr-iconv, but I strongly recommend
that if you change something, get rid of the APR_ICONV_PATH (on windows at least), or at least
use it as the last option like TSVN does now.
>
>Stefan
>
>-- 
>       ___
>  oo  // \\      "De Chelonian Mobile"
> (_,\/ \_/ \     TortoiseSVN
>   \ \_/_\_/>    The coolest Interface to (Sub)Version Control
>   /_/   \_\     http://tortoisesvn.tigris.org



Mime
View raw message