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:[EMAIL PROTECTED] http://use.perl.org http://apacheweek.com http://modperlbook.org http://apache.org http://ticketmaster.com


--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]



Reply via email to