Philippe M. Chiasson wrote:


Stas Bekman wrote:

Philippe M. Chiasson wrote:


Stas Bekman wrote:


[EMAIL PROTECTED] wrote:


gozer 2004/09/09 15:16:38




+/* XXX: There is no XS accessible splice() */
+static void modperl_av_remove_entry(pTHX_ AV *av, I32 index)
+{
+    I32 i;
+    AV *tmpav = newAV();
+
+    /* stash the entries _before_ the item to delete */
+    for (i=0; i<=index; i++) {
+        av_store(tmpav, i, SvREFCNT_inc(av_shift(av)));
+    }
+     +    /* make size at the beginning of the array */
+    av_unshift(av, index-1);
+     +    /* add stashed entries back */
+    for (i=0; i<index; i++) {
+        av_store(av, i, *av_fetch(tmpav, i, 0));
+    }
+     +    SvREFCNT_dec(tmpav);




shouldn't it just be sv_free'd? how do you know when the enclosing scope will force the free'ing when you can safely free it right here.



I was under the impression that SV *av = newAV(); [...] SvREFCNT_dec(av);

would achieve just that.



Yes, but with what price:

#if defined(__GNUC__) && !defined(__STRICT_ANSI__) && !defined(PERL_GCC_PEDANTIC)
# define SvREFCNT_dec(sv) \
({ \
SV *_sv = (SV*)(sv); \
if (_sv) { \
if (SvREFCNT(_sv)) { \
if (--(SvREFCNT(_sv)) == 0) \
Perl_sv_free2(aTHX_ _sv); \
} else { \
sv_free(_sv); \
} \
} \
})
#else


Are you saying you'd want to see:
SV *av = newAV();
[...]
av_undef(av);



SV *av = newAV(); ... sv_free(av).

won't that work just as well. I guess you get not much speed improvement here, but it's clear that this is a temp and you free it right there.


I read perlapi and basically got the idea that the only _clean_ way to destroy
a SV * is by decreasing it's refcnt


       sv_free Decrement an SV’s reference count, and if it drops to zero,
               call "sv_clear" to invoke destructors and free up any memory
               used by the body; finally, deallocate the SV’s head itself.
               Normally called via a wrapper macro "SvREFCNT_dec".

sv_clear
Clear an SV: call any destructors, free up any memory used by
the body, and free the body itself. The SV’s head is not freed,
although its type is set to all 1’s so that it won’t inadver-
tently be assumed to be live during global destruction etc.
This function should only be called when REFCNT is zero. Most
of the time you’ll want to call "sv_free()" (or its macro wrap-
per "SvREFCNT_dec") instead.


So I thought calling either sv_clear/sv_free directly wasn't recommended.

But here you know exactly that you have nothing else referencing the sv. I believe that warning in sv_clear about refcnt=0 has to do with the possibility of other svs referencing it. If you look at the perl internals, all the short functions with temp svs are written in this way.


I don't really care either way though.

In fact recently I first wrote this function as you did;

/* prepends the passed sprintf-like arguments to ERRSV, which also
 * gets stringified on the way */
void modperl_errsv_prepend(pTHX_ const char *pat, ...)
{
    SV *sv;
    va_list args;

    va_start(args, pat);
    sv = vnewSVpvf(pat, &args);
    va_end(args);

    sv_catsv(sv, ERRSV);
    sv_copypv(ERRSV, sv);
    sv_free(sv); /* was: SvREFCNT_dec(sv) */
}

when I showed it to Rafael, he said why don't you use sv_free instead of SvREFCNT_dec.

--
__________________________________________________________________
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