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

Re: Ruby test failure

From: Malcolm Rowe <malcolm-svn-dev_at_farside.org.uk>
Date: 2006-03-18 14:43:45 CET

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

This is an archived mail posted to the Subversion Dev mailing list.