On Fri, Mar 17, 2006 at 03:16:38PM +0900, Kouhei Sutou wrote:
> > OK, thanks for the answer. This makes me think that backporting the 1.3.28
> > support into 1.3.1 is a bit scary, at least for Ruby. Shouldn't we
> > either disallow Ruby support for Swig 1.3.28 or do something about the
> > test failure?
>
> I fixed the problem in r18924.
>
Hi Kou,
I was trying to review r18924 so that I could approve it for backport to
1.3.2, but I'm not sure I understand what it's doing.
The relevant change was in svn_swig_rb_get_pool(), which (as far as I
can infer) is responsible for converting a (possibly implicitly created)
Ruby pool object to an apr_pool_t*.
The relevant parts of the function look like this:
354 void
355 svn_swig_rb_get_pool(int argc, VALUE *argv, VALUE self,
356 VALUE *rb_pool, apr_pool_t **pool)
357 {
358 VALUE target;
359 apr_pool_wrapper_t *pool_wrapper;
360 apr_pool_wrapper_t **pool_wrapper_p;
...
*rb_pool = {various}
...
389
390 pool_wrapper_p = &pool_wrapper;
391 SWIG_ConvertPtr(*rb_pool, (void **)pool_wrapper_p,
392 SWIG_TypeQuery("apr_pool_wrapper_t *"), 0);
393 *pool = pool_wrapper->pool;
394 }
So the change you made in r18924 was just to change the 'flags' parameter
passed to SWIG_ConvertPtr() from non-zero to zero. That parameter is
explained as follows in the SWIG documentation:
int SWIG_ConvertPtr(VALUE obj, void **ptr, swig_type_info *ty, int flags)
Converts a Ruby object obj to a C pointer whose address is ptr (i.e. ptr
is a pointer to a pointer). The third argument, ty, is a pointer to a
SWIG type descriptor structure. If ty is not NULL, that type information
is used to validate type compatibility and other aspects of the type
conversion. If flags is non-zero, any type errors encountered during
this validation result in a Ruby TypeError exception being raised;
if flags is zero, such type errors will cause SWIG_ConvertPtr() to
return -1 but not raise an exception. If ty is NULL, no type-checking
is performed.
So by changing 'flags' to zero, we change the behaviour of
SWIG_ConvertPtr() when passed a Ruby object that's not a Ruby pool,
from raising an exception (of some sort; is this like a C++ exception
that skips the remainder of the function?) to just returning -1. As we
don't check the return value, we'll essentially ignore the problem.
My questions are:
* What guarantee do we have that pool_wrapper is dereferencable at all,
when we aren't checking the return value of the function that sets it?
I don't see any guarantee that SWIG_ConvertPtr() ensures that the
output pointer is set to anything valid on error.
* What problem is this fixing? The log message says "Support SWIG 1.3.28.
Keep owning pool.", but apart from fixing the Ruby test failures
(somehow), I don't understand what this change is trying to achieve.
Regards,
Malcolm
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Mar 18 14:44:10 2006