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

Re: svn commit: r12144 - trunk/subversion/bindings/swig/perl/libsvn_swig_perl

From: Ben Reser <ben_at_reser.org>
Date: 2004-12-21 20:35:47 CET

On Sat, Dec 04, 2004 at 09:13:59AM -0600, clkao@tigris.org wrote:
> Author: clkao
> Date: Sat Dec 4 09:13:55 2004
> New Revision: 12144
>
> Modified:
> trunk/subversion/bindings/swig/perl/libsvn_swig_perl/swigutil_pl.c
> Log:
> r11300@ab: clkao | 2004-11-17T15:48:54.151638Z
> * Pass pool to methods in Perl for close_baton.
> * Use a hash to cache SWIG_TypeQuery results.
>
> * libsvn_swig_perl/swigutil_pl.c:
> (close_baton): Pass pool to methods in Perl.
> Use svn_swig_pl_callback_thunk.
>
> (_swig_perl_type_query): New.
> (_SWIG_TYPE): New.
>
> Change callers of SWIG_TypeQuery to use _swig_perl_type_query.
>
>
>
> Modified: trunk/subversion/bindings/swig/perl/libsvn_swig_perl/swigutil_pl.c
> Url: http://svn.collab.net/viewcvs/svn/trunk/subversion/bindings/swig/perl/libsvn_swig_perl/swigutil_pl.c?view=diff&rev=12144&p1=trunk/subversion/bindings/swig/perl/libsvn_swig_perl/swigutil_pl.c&r1=12143&p2=trunk/subversion/bindings/swig/perl/libsvn_swig_perl/swigutil_pl.c&r2=12144
> ==============================================================================
> --- trunk/subversion/bindings/swig/perl/libsvn_swig_perl/swigutil_pl.c (original)
> +++ trunk/subversion/bindings/swig/perl/libsvn_swig_perl/swigutil_pl.c Sat Dec 4 09:13:55 2004
> @@ -31,6 +31,32 @@
>
> #include "swigutil_pl.h"
>
> +/* cache SWIG_TypeQuery results in a perl hash */
> +static HV *type_cache = NULL;
> +
> +#define _SWIG_TYPE(name) _swig_perl_type_query(name, sizeof (name)-1)

The above macro is bad. It references the paramter name twice. You
shouldn't do that. In this particular case it doesn't break anything
from the existing usages. But it might in the future.

Additionally _swig_perl_type_query wants the strlen not the size of the
pointer. Probably should just use:

#define _SWIG_TYPE(name) _swig_perl_type_query(name, 0);

since....

> +#define POOLINFO _SWIG_TYPE("apr_pool_t *")
> +
> +static swig_type_info *_swig_perl_type_query (const char *typename, U32 klen)
> +{
> + SV **type_info;
> + swig_type_info *tinfo;
> +
> + if (!type_cache)
> + type_cache = newHV ();
> +
> + if (klen == 0)
> + klen = strlen (typename);

You've already allowed 0 to be passed to the function to calculate the
strlen for you. No since in putting it in the macro then. Especially
since you have to jump through hoops to make that a safe macro.

> + if ((type_info = hv_fetch(type_cache, typename, klen, 0)))
> + return (swig_type_info *) (SvIV (*type_info));
> +
> + tinfo = SWIG_TypeQuery(typename);
> + hv_store(type_cache, typename, klen, newSViv ((IV)tinfo), 0);
> +
> + return tinfo;
> +}

[snip] rest of it looks fine.

-- 
Ben Reser <ben@reser.org>
http://ben.reser.org
"Conscience is the inner voice which warns us somebody may be looking."
- H.L. Mencken
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Dec 21 20:42:27 2004

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.