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

Re: Badness in mod_dav_svn?

From: John Szakmeister <john_at_szakmeister.net>
Date: 2005-12-11 22:45:00 CET

On Sunday 11 December 2005 13:51, you wrote:
> Well, thanks to our excellent log message format, you can see the
> history of this function. Looks like it was created by gstein way
> back in the early days, then complexified a bit by me in r405 and
> cmpilato in r2090. But the svn_error_clear() bit was added by
> julianfoad in r8923.
> Of course, at no point have errors ever been *handled* (i.e. reported
> or acted upon) in the history of this routine. Julian was simply
> trying to stop us from leaking the ignored errors.

The first version didn't do nearly as much work (it simply closed the second
resource and assigned the repos pointer from the first resource into the
second). That changed pretty dramatically in r405 when we started opening
the txn by name and regenerating the root object.

Either way is doesn't matter much. Ignoring errors is bad, especially when
there are going to be lingering references to objects that are unrelated to
the repos parameter being stored in the resource.

What's the purpose of coalescing this stuff together this way? It doesn't
seem like the place to do this is in a routine that's supposed to be checking
whether or not two resources are referring to the same thing, especially when
there is no way to propagate a real error out when something fails (short of
segfaulting, and that seems like a Bad Thing To Do on a server). IOW, I
don't think the intention of the function was to edit the resources and
coalesce them. Looking at mod_dav_fs's implementation further validates that

Can we get rid of the coalescing? Does it affect performance in some way? Is
the better answer to hack mod_dav and have mod_dav help to coalesce this
information via some other hook?

FWIW, I removed the code from is_our_resource() altogether and the test suite


To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sun Dec 11 22:46:24 2005

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.