Justin Erenkrantz <jerenkrantz@apache.org> writes:
> On platforms without iconv (*BSDs), 'svn log' exits with a fatal
> error when a unconvertable log message is encountered. It has to
> do with check_non_ascii returning SVN_ERR_UNSUPPORTED_FEATURE when
> it encounters this case and log_message_receiver only checking for
> APR_EINVAL.
>
> So, there are two solutions:
>
> 1) Alter check_non_ascii to return APR_EINVAL.
> 2) Have all callers of the utf conversion functions check for
> APR_EINVAL *and* SVN_ERR_UNSUPPORTED_FEATURE.
>
> My vote is for #1. The crew on #svn seemed much more in favor of #2.
> I believe it makes more sense to have a consistent API in this case.
> But, I just want it to work. =) -- justin
While I agree with the change to check_non_ascii(), it should be part
of an overall change that implements strategy #2. Let me explain:
Taking check_non_ascii() as an independent interface, it makes little
sense for it to return UNSUPPORTED_FEATURE on finding "unsafe" data.
APR_EINVAL is a much more appropriate error. So +1 on that part of
the change.
Nevertheless, callers of the public utf conversion functions *should*
be able to distinguish between these two cases:
1. "We tried to convert the data, but some of it is not
representable in your locale."
versus
2. "We don't do conversion at all, and make no claims about whether
the data is representable in your locale."
Therefore, the direct callers of check_non_ascii() should check its
return value, and convert the error to SVN_ERR_UNSUPPORTED_FEATURE as
appropriate, so their callers can make this distinction.
I certainly agree that either strategy would serve to address your
issue above, of course. I'm thinking about the longer term, and have
a hunch that we may later want/need this knowledge, so the API should
not toss the information.
Note that your point (2)
> 2) Have all callers of the utf conversion functions check for
> APR_EINVAL *and* SVN_ERR_UNSUPPORTED_FEATURE.
...is not as serious as it sounds. Most callers don't check for a
specific error at all, they just handle all errors the same way. The
big exception to this is log_message_receiver(), because it's for
human consumption (as Uli pointed out), and it will have to be
sensitive in several ways. Checking for two documented error codes
instead of one will be the least of our worries; I don't think it's a
big burden.
Which brings me to my next point:
I'm planning to resolve both your issue above and related issue #807
by incorporating your patch below, plus some code from Ulrich Drepper,
plus some changes of my own. That's a P1, so look for it in the next
few days.
If any objections to the above plan, please post! Thanks,
-K
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sun Jul 28 15:11:13 2002