[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: started applying Marcus' patch

From: Marcus Comstedt <marcus_at_mc.pp.se>
Date: 2002-07-09 21:36:38 CEST

Karl Fogel <kfogel@newton.ch.collab.net> writes:

> By the way, Marcus --
>
> We haven't yet applied your patch to APR's xlate.c, about which you
> wrote the following in a mail message once:
>
> Implementation note: I'm now using apr_xlate instead of calling
> iconv directly. This means that it's now up to APR to worry about
> portability. Unfortunately, apr_xlate seems to make several of the
> mistakes that Ulrich cautioned about. In particular, it ignores
> return codes greater than 0 from iconv. It should probably return
> some other apr_status_t than APR_SUCCESS in this case, to indicate
> an inexact translation. Of course, we'll need to decide on in
> which cases an inexact translation is acceptable as well. For a
> pathname, probably not. For an error message, probably. But what
> about a property diff, for example?
>
> Have you had any more thoughts on that?

This is unrelated to my APR patch, which is just something to avoid
the call to iconv_open if both charsets are the same. (That's not
just an optimization, at least on Solaris iconv_open can actually fail
if you request a converter from UTF-8 to UTF-8...)

Regarding the inexact translations, I tried to find an appropriate APR
error code to use, but since APR_INCOMPLETE has already been allocated
to mean that the input ends with an unterminated multibyte sequence,
there weren't any really fitting candidate. Maybe it's ok to just
create a new error code APR_INEXACT?

As for the policy regarding inexact translations in svn, I think that
pathnames must always be exactly translated to avoid aliasing
problems, and that error messages should be allowed to be inexact
since an error message that just says that the error message could not
be printed is about as unhelpful as you can get. :-)

When _setting_ a property or log message, exact conversion should
probably be required. Maybe it might make sense to be more lax when
_getting_ them, at least for log messages, but I'm not too sure about
that.

These are policy questions, so they have to be discussed a bit more.
For the moment, only inexact translation is offered by apr_xlate
anyway, so there's no need to rush.

> The patch itself is below. Can you write a log message for it? I
> don't think there was one included with the patch, and didn't see one
> posted later. (I don't understand everything the patch does just from
> looking at it, though perhaps someone more knowledgable than I about
> APR's i18n support would be able to get by without a log message).

As I said, this patch only fixes the unity converson problem. Log
message:

* i18n/unix/xlate.c
  (make_identity_table): New function to create a sbcs_table that
performs unity conversion. See apr_xlate_open.
  (apr_xlate_open): When topage and frompage are the same,
make_identity_table is used to create a shortcut instead of calling
iconv_open. The reason being that iconv_open can fail in this case.

Shouldn't we CC some APR list at this point?

  // Marcus

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Jul 9 21:42:58 2002

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.