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

Re: svn commit: r15370 - in branches/python-bindings-improvements/subversion/bindings/swig: python/libsvn_swig_py python/svn

From: David James <james82_at_gmail.com>
Date: 2005-07-20 01:29:42 CEST

On 7/19/05, Philip Martin <philip@codematters.co.uk> wrote:
> > +static char *poolAttribute = (char *) "_pool";
> > +static char *assertValid = (char *) "assert_valid";
> > +static char *emptyTuple = (char *) "()";
>
> Casting away const is generally a bad idea, how about
> static char name[] = "value";
Good catch! I'll switch.

> > +/*** Automatic Pool Management Functions ***/
> > +
> > +/* The application pool */
> > +apr_pool_t *_global_pool = NULL;
> That's not a good name for something that is globally visible, how about
> svn_swig_py_application_pool?
> [...snip...]
> Given that function interface, why is _global_pool extern and not
> static?

Good question! Normally, it is not a good idea to create globally
visible variables, especially when such variables can already be
accessed and modified by globally visible functions.

However, because of our SWIG bindings, we are faced with a special
situation: Many of the SWIG bindings access the current pool by
accessing a variable called "_global_pool" . If your current function
has access to a local pool, then this technique will work. On the
other hand, if the function needs a pool but does not have one, you
will get errors stating that the compiler cannot resolve the variable
"_global_pool". Currently, we resolve this issue by excluding all
functions which need pools but do not have them from the SWIG
bindings.

In order to resolve the above problem, I created a global variable
with the same name as the local variable "_global_pool". If the local
variable "_global_pool" is not defined, then we will use the global
variable "_global_pool" instead. With this new technique, it is now
possible to compile functions which need pools but do not have them.

To make my code more clear, I propose to take the following steps:
- Rename the _global_pool variable to svn_swig_application_pool
throughout the SWIG
  bindings (including in the Perl and Ruby bindings)
- Add a comment to the source code explaining why I created a global variable.

Philip, does this sound like a good solution?

Cheers,

David

-- 
David James -- http://www.cs.toronto.edu/~james
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Jul 20 01:30:51 2005

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