Mailing-List: contact dev-help@perl.apache.org; run by ezmlm
Precedence: bulk
Message-ID: <3EDC4E89.7010806@stason.org>
Date: Tue, 03 Jun 2003 17:30:17 +1000
From: Stas Bekman <stas@stason.org>
Organization: Hope, Humanized
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.1) Gecko/20020826
MIME-Version: 1.0
To: Geoffrey Young <geoff@modperlcookbook.org>
Cc: dev@perl.apache.org
Subject: Re: opening up $cfg to XS
References: <3ED7662C.2030303@modperlcookbook.org>
 <3ED812FE.7030609@stason.org> <3EDB5927.9020400@modperlcookbook.org>
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit

Geoffrey Young wrote:
> 
>>
>> - if your wrapper calls directly the function, without doing anything 
>> else, but dropping "self" you should use DEFINE_, just in case 
>> compiler doesn't inline that call. I don't remember if it's in the doc.
> 
> 
> re this an SvREFCNT_inc below, I figured it might be best to keep it as 
> a separate function and not use define.  in the patch below, the C 
> function digs out the object, while the *.h wrapper calls the C function 
> and increments the refcount before returning to Perl-land.  sound ok?

but this can still be done in define:

#define ... SvREFCNT_inc(modperl_module_config_get(aTHX_ pmodule, s, v));

no?

>> Probably need to add comments explaining the difference between this 
>> and get_config_obj functions.
> 
> 
> as I read things, get_config_obj isn't digging out the object, it's 
> creating it.  in fact, if the object already exists it doesn't even 
> return it
> 
>     if ((*obj = (SV*)modperl_svptr_table_fetch(aTHX_ table, cfg))) {
>         /* object already exists */
>         return NULL;
>     }
> 
> so, the below patch renames that function to 
> modperl_module_config_create_obj instead, in the hopes of clarifying 
> things a bit more.

+1

shouldn't then the new function be called modperl_module_config_get_obj?

>> - Also I'm not sure about keeping the SvREFCNT_inc(obj) part. It's 
>> needed when returning an SV that goes into perl domain, but it's 
>> probably not a good choice for C API.
> 
> 
> anyway, lemme know if you think this fits.

please remember to indent the args where the function name is now longer once 
we agree on the change


__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org