Greg Stein <gstein@lyra.org> writes:
> On Tue, Jun 12, 2001 at 08:48:58PM -0000, sussman@tigris.org wrote:
> > User: sussman
> > Date: 01/06/12 13:48:58
> >
> > Modified: subversion/libsvn_subr svn_error.c
> > Log:
> > * svn_error.c (initialize_svn_error_descriptions): new func - build a
> > hash table that maps svn errorcodes to english descriptions.
> >
> > (svn_strerror): take a pool instead of stringbuf. pass
> > this pool to initialize_svn_error_descriptions() the first time this
> > routine is called, and then cache the hash in a static variable
> > thereafter. use the hash for lookups.
> >
> > (svn_error_strings[]): removed, now part of hash initialization.
>
> This isn't going to work well.
And Ben rebuts! Karl spent a long time arguing about how to solve
this problem. We were waiting for someone to attack. ;)
>
> If the first person to call svn_strerror() passes a subpool, then future
> callers will get garbaged if that pool has gone away. It is fixable by using
> a cleanup so the hash can be rebuilt for another caller.
There is only one caller: svn_handle_error, which always uses the
pool within the svn_error_t, which is the top-level pool.
So -- we either need to make svn_strerror a static routine (so that
ONLY svn_handle_error can use it), or we need to document that the
pool must be top-level pool, or we need
>
> However, this is also overengineered. We don't need a hash table since this
> function won't be called often. A simple linear scan over a table is
> sufficient. That would allow constant data to be used for the whole table.
You miss the point of the hash table.
We expect a lot of skew. At first, we thought, "let's avoid the skew"
by forcing all new svn errors & descriptions to be put into a single
python list -- and then at build time, python can output a mini .h and
.c file to be #included in svn_error.h and svn_error.c. (This is what
a lot of big projects do.)
Then we came up with a solution we like better: instead of preventing
skew, make it harmless. We expect people to add a new svn errorcode
in svn_error.h, and then either forget to add a description in
svn_error.c, or perhaps add the descripiton in the wrong place. If
the descriptions were in a plain old array (as it used to be), then
either of these human errors would cause ALL the errors to be screwed
up, a big fat OBOE.
The point of having a hash is that order doesn't matter. Either of
the two mistakes don't matter. svn_strerror() just returns "no
description available" if the code isn't in the hash. And it doesn't
matter where (in the source) the new description is hash-set.
> And lastly, it would be nice to just call svn_strerror() and have it grab
> the APR error for us, if it falls in the APR range. No need for callers to
> know everything about the ranges.
We can do that, easily. That logic is currently in svn_handle_warning
right now, but we can move it.
>
>
> I've got a design for rectifying all of this, and can implement tonite. I
> explained the evil macros to Karl if you can't wait to see the code
> :-)
Oh, oops. Have you already talked Karl out of this system? Then I
guess my comments are irrelevant. :-)
>
> [ the gist is to have a header that can create both the enumeration and the
> error string table, depending on how it is included ]
Whoa. My head hurts.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Oct 21 14:36:31 2006