On Thu, 26 Aug 2004, Greg Hudson wrote:
> Your docstring for svn_utf_initialize says:
>
> +/** @since New in 1.1.
> + * Initialize the utf8 encoding/decoding routines.
> + * NOTE: It is optional to call this function, but if it is used, no other
> + * svn_utf function may be in use during the call of this function.
> + * Initializing the UTF-8 routines will improve performance.
> + */
> +void svn_utf_initialize (apr_pool_t *pool);
>
> There was a reason I said "no Subversion library function may be in use"
> when I described the constraint; it's clearer to describe it that way.
> Almost any Subversion library function is liable to use svn_utf
> functions.
>
I changed it to require that no svn function may be active during this
call. So you can create a pool before this call, which is important for
cleanup order and performance.
> If you're going to take a pool parameter, document that the pool must
> live for as long as Subversion library functions are in use.
> (Another option is to create a pool from scratch like we do in
> svn_error_create(). But on consideration, I think that's a bad idea,
> since that pool could never be cleaned up.)
>
OK. The docstring wasn't ready yet. Really, I wanted to get feedback on
the approach more than the exact patch, but I could have been clearer on
that... I removed the pool, since we need to create a subpool anyway,
which we can protect with the mutex. Pools created with no parent are
really owned by an internal pool in APR which will be destroyed during
apr_terminate. The patch works even if something uses UTF-8 translations
after the cache is cleaned up, but then without the cache ofcourse. I'm
assuming that all other threads are finished when apr_terminate gets
called. This must be required anyway...
> + if (utf_initialized++ == 0)
>
> If you're treating utf_initialized as a flag, then it's stylistically
> confusing to operate on it like it's a counter. APR treats its
> initialization static as a counter because it wants to count
> apr_terminate() calls to know when it should clean up.
>
You're right. I removed the flag completely and use the hash table pointer
as a flag instead.
> I'll review more thoroughly when you've written a log message. It's
> good to do that before asking for review.
>
You're right again. As I said, I asked more about the concept, but
expecting you to read my mind might be to require too much:-)
This time, it would be nice with some review. This got complicated, at
least for me...
Thanks,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Aug 27 22:43:50 2004