Kouhei Sutou wrote:
> "C. Michael Pilato" <cmpilato_at_collab.net> wrote:
>
> > wouldn't expect this to actually remedy a pool lifetime problem. So, if we
> > can't explain exactly why your patch helps and what problem it solves, my
> > vote is -1 on applying it. We don't want to make the tests stop failing; we
> > want to make Subversion work correctly (and we'll happily accept passing
> > tests as a benefit of that change).
>
> I think that "someone says the fix is not correct" or "you
> say the tests are wrong" is needed before you vote -1.
I would say that if any change has no explanation for why it should
work, and appears to be able to "fix" only the symptom and not the
problem, that is sufficient grounds to say it is wrong and to veto it.
> I'm sorry but I don't have time to look into the problem
> now. There are a code that reproduces the problem and a
> patch that fixes (or hides) the problem. It seems that
> someone can look into the problem.
It's OK that you don't have time to look into it now.
> (My expectation WITHOUT foundation: I think that
> svn_txdelta_send_stream() destroys passed pool
> somewhere. The passed pool is not top-level pool
> (svn_pool_create(parent_pool)) normally but it may be
> top-level pool (svn_pool_create(NULL)) in the Ruby bindings
> test. The difference may cause the problem.)
I had a quick look in libsvn_delta and couldn't find the problem. I
noticed the pool usage in libsvn_delta is far from straightforward, so I
couldn't follow it to know whether it is correct.
A pool-lifetime comment like this, and more, would be useful:
[[[
Index: subversion/include/svn_delta.h
===================================================================
--- subversion/include/svn_delta.h (revision 35773)
+++ subversion/include/svn_delta.h (working copy)
@@ -410,10 +410,11 @@
/** Prepare to apply a text delta. @a source is a readable generic stream
* yielding the source data, @a target is a writable generic stream to
* write target data to, and allocation takes place in a sub-pool of
* @a pool. On return, @a *handler is set to a window handler function and
* @a *handler_baton is set to the value to pass as the @a baton argument to
+ * @a *handler. @a pool must survive through all subsequent calls to
* @a *handler.
*
* If @a result_digest is non-NULL, it points to APR_MD5_DIGESTSIZE bytes
* of storage, and the final call to @a handler populates it with the
* MD5 digest of the resulting fulltext.
]]]
- Julian
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1130689
Received on 2009-02-09 23:03:51 CET