You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Branko Čibej <br...@xbc.nu> on 2002/07/18 22:59:32 UTC
Re: svn commit: rev 2586 - trunk trunk/subversion/libsvn_subr
kfogel@tigris.org wrote:
>Modified: trunk/subversion/libsvn_subr/utf.c
>==============================================================================
>--- trunk/subversion/libsvn_subr/utf.c (original)
>+++ trunk/subversion/libsvn_subr/utf.c Thu Jul 18 17:26:40 2002
>@@ -32,13 +32,15 @@
> #include "svn_utf.h"
>
>
>-#ifdef SVN_UTF8
>
> #define SVN_UTF_NTOU_XLATE_HANDLE "svn-utf-ntou-xlate-handle"
> #define SVN_UTF_UTON_XLATE_HANDLE "svn-utf-uton-xlate-handle"
>
> /* Return the apr_xlate handle for converting native characters to UTF-8.
>- Create one if it doesn't exist. */
>+ Create one if it doesn't exist. If unable to find a handle, or
>+ unable to create one because apr_xlate_open returned EINVAL, then
>+ set *RET to null and return SVN_NO_ERROR; if fail for some other
>+ reason, return error. */
> static svn_error_t *
> get_ntou_xlate_handle (apr_xlate_t **ret, apr_pool_t *pool)
> {
>@@ -68,7 +70,13 @@
> /* Try to create one. */
> apr_err = apr_xlate_open (ret, "UTF-8", APR_LOCALE_CHARSET, global_pool);
>
>- if (apr_err != APR_SUCCESS)
>+ /* apr_xlate_open returns EINVAL if no handle could be found. */
>+ if (apr_err == EINVAL)
>
APR_STATUS_IS_EINVAL(apr_err), please.
>+ {
>+ *ret = NULL;
>+ return SVN_NO_ERROR;
>+ }
>+ else if (apr_err != APR_SUCCESS)
> return svn_error_create (apr_err, 0, NULL, pool,
> "failed to create a converter to UTF-8");
>
>@@ -81,7 +89,10 @@
>
>
> /* Return the apr_xlate handle for converting UTF-8 to native characters.
>- Create one if it doesn't exist. */
>+ Create one if it doesn't exist. If unable to find a handle, or
>+ unable to create one because apr_xlate_open returned EINVAL, then
>+ set *RET to null and return SVN_NO_ERROR; if fail for some other
>+ reason, return error. */
> static svn_error_t *
> get_uton_xlate_handle (apr_xlate_t **ret, apr_pool_t *pool)
> {
>@@ -111,6 +122,12 @@
> /* Try to create one. */
> apr_err = apr_xlate_open (ret, APR_LOCALE_CHARSET, "UTF-8", global_pool);
>
>+ /* apr_xlate_open returns EINVAL if no handle could be found. */
>+ if (apr_err == EINVAL)
>
Same here.
--
Brane Čibej <br...@xbc.nu> http://www.xbc.nu/brane/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: rev 2586 - trunk trunk/subversion/libsvn_subr
Posted by Nuutti Kotivuori <na...@iki.fi>.
Karl Fogel wrote:
> Justin Erenkrantz <je...@apache.org> writes:
>> > I wish. However, currently the functions return EINVAL, not
>> > APR_EINVAL. The APR_STATUS_IS_EINVAL macro only tests for the
>> > latter, I thought.
>>
>> On systems where EINVAL is defined (i.e. where the old code
>> compiled), APR_EINVAL is just #define'd to EINVAL.
>>
>> APR is full of neato-tricks like that. -- justin
>
> So on other systems, the APR_STATUS_IS_EINVAL(apr_status) macro can
> return true not only when
>
> apr_status == APR_EINVAL (but not necessarily equal to EINVAL)
>
> ...but also when it matches certain other conditions.
>
> In other words, it's fine for apr_xlate_open function to return
> APR_EINVAL instead of EINVAL, but if that precise value is to signal
> a specific condition, then callers should use a direct equality test
> instead of the APR_STATUS_IS_EINVAL() macro. Or so I would think?
Ouchie. Hey, I'm not the APR guru here, you guys are. But I still
think this is not the way to go here.
First the couple facts:
- APR_EINVAL can either be equal to EINVAL, or not.
- APR_STATUS_IS_EINVAL(s) checks whether s is EINVAL or APR_EINVAL,
but does check for other values as well.
So, the first conclusion to come from this is:
- Hey, if I return APR_EINVAL explicitly and check for APR_EINVAL
explicitly, I will detect my precise condition for returning it!
But the obvious counter-conclusion that comes a bit later:
- APR_EINVAL can be EINVAL for real, so I might assume a system
returned error as my precise condition on one platform and not on the
other. And even if APR_EINVAL and EINVAL are different, some APR
function might explicitly return APR_EINVAL.
So, even if no APR functions are called in this case, using APR_EINVAL
as an explicit test is just going to obfuscate things further if that
happens to be the case some day.
And, if there is a explicit condition to be returned by the function,
that should be returned as a new error/status code, that is guaranteed
not to have any double meanings.
But, I might be talking out of my ass, since I don't really know how
these are supposed to be used.
-- Naked
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: rev 2586 - trunk trunk/subversion/libsvn_subr
Posted by Karl Fogel <kf...@newton.ch.collab.net>.
Justin Erenkrantz <je...@apache.org> writes:
> > I wish. However, currently the functions return EINVAL, not
> > APR_EINVAL. The APR_STATUS_IS_EINVAL macro only tests for the latter,
> > I thought.
>
> On systems where EINVAL is defined (i.e. where the old code
> compiled), APR_EINVAL is just #define'd to EINVAL.
>
> APR is full of neato-tricks like that. -- justin
So on other systems, the APR_STATUS_IS_EINVAL(apr_status) macro can
return true not only when
apr_status == APR_EINVAL (but not necessarily equal to EINVAL)
...but also when it matches certain other conditions.
In other words, it's fine for apr_xlate_open function to return
APR_EINVAL instead of EINVAL, but if that precise value is to signal a
specific condition, then callers should use a direct equality test
instead of the APR_STATUS_IS_EINVAL() macro. Or so I would think?
-K
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: rev 2586 - trunk trunk/subversion/libsvn_subr
Posted by Justin Erenkrantz <je...@apache.org>.
On Fri, Jul 19, 2002 at 12:45:43PM -0500, Karl Fogel wrote:
> Branko ??ibej <br...@xbc.nu> writes:
> > >+ /* apr_xlate_open returns EINVAL if no handle could be found. */
> > >+ if (apr_err == EINVAL)
> > >
> >
> > APR_STATUS_IS_EINVAL(apr_err), please.
>
> I wish. However, currently the functions return EINVAL, not
> APR_EINVAL. The APR_STATUS_IS_EINVAL macro only tests for the latter,
> I thought.
On systems where EINVAL is defined (i.e. where the old code
compiled), APR_EINVAL is just #define'd to EINVAL.
APR is full of neato-tricks like that. -- justin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: rev 2586 - trunk trunk/subversion/libsvn_subr
Posted by Karl Fogel <kf...@newton.ch.collab.net>.
Karl Fogel <kf...@newton.ch.collab.net> writes:
> So the very existence of APR_STATUS_IS_SUCCESS() is controversial, and
> you and I may be fairly said to be on the "against" side, then? :-)
Everyone please ignore this question :-). I had NO intention of
starting a discussion about this. My original mail to Brane was
private; he accidentally replied to it publically, and then I replied
to his reply before I realized that the thread was no longer just
between him and me.
(If I knew it were the whole list, I would have gone back and dug up
the older conversation about APR_STATUS_IS_SUCCESS and re-read that
whole thing first. I'm happy to waste Brane's time, but I would never
waste *your* time :-) ).
-K
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: rev 2586 - trunk trunk/subversion/libsvn_subr
Posted by Karl Fogel <kf...@newton.ch.collab.net>.
Branko Čibej <br...@xbc.nu> writes:
> Well, technically yes, but that's so much more confusing that
> way. "Why're you comparing here, but using a macro there? I like the
> comparison better, let's throw out the macro ... cool, that still
> works ... hey, where are all those Win32 bug reports coming from?" You
> get the picture.
Yeah. I don't think it's ever going to cause us a problem in the
specific code under discussion here. But anyone who looks carefully
at the interfaces, and at the macro expansions, is going to scratch
their head. That's fine, we can answer them if they ask :-).
> You may presume to have supposed correctly.
I would venture to hazard a guess that our hypothesis is considered
worthy of further consideration by others who shall remain nameless.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: rev 2586 - trunk trunk/subversion/libsvn_subr
Posted by Branko Čibej <br...@xbc.nu>.
Karl Fogel wrote:
>Branko Čibej <br...@xbc.nu> writes:
>
>>The purpose of those macros is to hide platform differences. Which,
>>incidentally, is the purpose of APR. That's why you should always use
>>those macros instead of direct comparison, _except_ for APR_SUCCESS,
>>where it doesn't matter because it's guaranteed to be 0 on all
>>platforms.
>>
>>
>
>Or except when the apr function promises to return *precisely*
>APR_EINVAL (to pick a random example) under certain circumstances, and
>the caller needs to look for that circumstance?
>
Well, technically yes, but that's so much more confusing that way.
"Why're you comparing here, but using a macro there? I like the
comparison better, let's throw out the macro ... cool, that still works
... hey, where are all those Win32 bug reports coming from?" You get the
picture.
>I dunno. I'm using the macro test anyway right now in Subversion,
>despite the above.
>
+1
>So the very existence of APR_STATUS_IS_SUCCESS() is controversial, and
>you and I may be fairly said to be on the "against" side, then? :-)
>
>
You may presume to have supposed correctly.
--
Brane Čibej <br...@xbc.nu> http://www.xbc.nu/brane/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: rev 2586 - trunk trunk/subversion/libsvn_subr
Posted by Karl Fogel <kf...@newton.ch.collab.net>.
Justin Erenkrantz <je...@apache.org> writes:
> > Or except when the apr function promises to return *precisely*
> > APR_EINVAL (to pick a random example) under certain circumstances, and
> > the caller needs to look for that circumstance?
>
> Nope, just use APR_STATUS_IS_EINVAL and repeat, "This is the way to
> do it."
Well, that's a very *clear* answer, but it's not very helpful :-), as
it doesn't actually address the concern I laid out in my mail...
-K
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: rev 2586 - trunk trunk/subversion/libsvn_subr
Posted by Justin Erenkrantz <je...@apache.org>.
On Fri, Jul 19, 2002 at 05:46:01PM -0500, Karl Fogel wrote:
> Or except when the apr function promises to return *precisely*
> APR_EINVAL (to pick a random example) under certain circumstances, and
> the caller needs to look for that circumstance?
Nope, just use APR_STATUS_IS_EINVAL and repeat, "This is the way to
do it."
> So the very existence of APR_STATUS_IS_SUCCESS() is controversial, and
> you and I may be fairly said to be on the "against" side, then? :-)
The sad fact is I don't think anyone is on the for side. -- justin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: rev 2586 - trunk trunk/subversion/libsvn_subr
Posted by Karl Fogel <kf...@newton.ch.collab.net>.
Branko Čibej <br...@xbc.nu> writes:
> Who's that "everyone"? Everyone insists that
> APR_STATUS_IS_SUCCESS(foo) is nonsense, because that is _guaranteed_
> to be the same as (foo == APR_SUCCESS), which is also guaranteed to be
> (!foo).
>
> The same everyone insists that the macros should always be used for
> checking everything except success.
I'm probably confusing APR_SUCCESS arguments with the more general
question of APR_*, yeah.
> >Perhaps there's a good explanation for all this, and I just haven't
> >been enlightened yet.
> >
> Mu! :-)
:-)
Fair enough!
> The purpose of those macros is to hide platform differences. Which,
> incidentally, is the purpose of APR. That's why you should always use
> those macros instead of direct comparison, _except_ for APR_SUCCESS,
> where it doesn't matter because it's guaranteed to be 0 on all
> platforms.
Or except when the apr function promises to return *precisely*
APR_EINVAL (to pick a random example) under certain circumstances, and
the caller needs to look for that circumstance?
I dunno. I'm using the macro test anyway right now in Subversion,
despite the above.
So the very existence of APR_STATUS_IS_SUCCESS() is controversial, and
you and I may be fairly said to be on the "against" side, then? :-)
-K
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: rev 2586 - trunk trunk/subversion/libsvn_subr
Posted by Branko Čibej <br...@xbc.nu>.
Karl Fogel wrote:
>>>APR_EINVAL. The APR_STATUS_IS_EINVAL macro only tests for the latter,
>>>I thought.
>>>
>>>
>>Yeah, right. And APR_EINVAL is defined to be EINVAL. apr_errno.h is your friend.
>>May the source be with you!
>>
>>
>
>But APR_STATUS_IS_EINVAL() can test for more than just APR_EINVAL...
>
>?? _sigh_
>
>I fscking *hate* those STATUS_IS macros. They obfuscate everything,
>because their definitions are non-trivial, but then everyone insists
>they're "just like" doing a simple equality test.
>
Who's that "everyone"? Everyone insists that APR_STATUS_IS_SUCCESS(foo)
is nonsense, because that is _guaranteed_ to be the same as (foo ==
APR_SUCCESS), which is also guaranteed to be (!foo).
The same everyone insists that the macros should always be used for
checking everything except success.
> If they were, we
>wouldn't have the macros. If they're not, then you can't use them as
>a simple equality test.
>
>Perhaps there's a good explanation for all this, and I just haven't
>been enlightened yet.
>
Mu! :-)
It's extremely simple.
* Every APR_STATUS_IS_E* macro tests at least the equivalent APR_E*
code.
* Where the POSIX/ISO C E* code exists, APR_E* is defined in terms
of it.
* There Be Platforms where there can be more than one system error
code that's semantically equivalent to one of the APR_E* codes
(and those are descended from POSIX/ISO C E* codes). On those
platforms, the APR_STATUS_IS_* macros must test more than one value.
The purpose of those macros is to hide platform differences. Which,
incidentally, is the purpose of APR. That's why you should always use
those macros instead of direct comparison, _except_ for APR_SUCCESS,
where it doesn't matter because it's guaranteed to be 0 on all platforms.
Are you enlightened yet? :-)
--
Brane Čibej <br...@xbc.nu> http://www.xbc.nu/brane/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: rev 2586 - trunk trunk/subversion/libsvn_subr
Posted by Karl Fogel <kf...@newton.ch.collab.net>.
Branko Čibej <br...@xbc.nu> writes:
> >+ /* apr_xlate_open returns EINVAL if no handle could be found. */
> >+ if (apr_err == EINVAL)
> >
>
> APR_STATUS_IS_EINVAL(apr_err), please.
I wish. However, currently the functions return EINVAL, not
APR_EINVAL. The APR_STATUS_IS_EINVAL macro only tests for the latter,
I thought.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org