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

Re: CVS update: MODIFIED: libsvn_subr ...

From: Ben Collins-Sussman <sussman_at_collab.net>
Date: 2001-06-14 01:18:11 CEST

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

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.