XS/guts Expert help needed

Matts on 2006-11-10T22:27:06

Anyone got any idea why the following leaks like a sieve:

char *
getline(str)
    SV * str
    CODE:
    {   
        STRLEN len;
        char *cval = SvPV(str, len);
        char *ret = (char*)calloc(len+1, sizeof(char));
        I32 taken = 0;
        if (!SvOK(str)) {
            XSRETURN_UNDEF;
        }
        if (SvTYPE(str) < SVt_PVIV)
            sv_upgrade(str,SVt_PVIV);
        RETVAL = ret;
        while (*cval) {
            *ret = *cval;
            if (*cval == '\n')
                break;
            *cval++; *ret++; taken++;
        }
        if (*cval == '\n') {
            *cval++; *ret++; taken++;
        }
        *ret = 0;
        if (!SvOOK(str)) {
            SvOOK_on(str);
            SvIVX(str) = 0;
        }
        SvIVX(str) += taken;
        SvPVX(str) += taken;
        SvLEN(str) -= taken;
        SvCUR(str) -= taken;
    }
    OUTPUT:
      RETVAL

It's designed to be a fast replacement for: s/^(.*?)\n//; in case you were wondering, and uses the rather underdocumented OOK hack in perl core (which allows you to quickly truncate off the start of a string by specifying an offset in the IV slot).

---

Re: XS/guts Expert help needed

jmcnamara on 2006-11-10T23:50:46

I'm guessing based on the C parts since I don't know XS very well.

Aren't you calloc()ing a char buffer without free()ing it.

John.
--

Re: XS/guts Expert help needed

Matts on 2006-11-11T00:01:54

Yes, but perl should free that when the SvREFCOUNT reaches zero.

Re: XS/guts Expert help needed

Matts on 2006-11-11T00:07:49

Never mind, that does indeed appear to be the culprit. How strange - I could have sworn perl free'd the strings.

Re: XS/guts Expert help needed

clkao on 2006-11-11T00:33:21

The string (ret) is copied from the svpv, and not maintained by any sv so it's not perl's responsibility to manage the memory allocated to it.

Re: XS/guts Expert help needed

Matts on 2006-11-11T00:50:02

What's weird is this pretty much guarantees that a char * typemap will leak (as you can't free it before you return) unless you are returning static char *s.

Re: XS/guts Expert help needed

demerphq on 2006-11-11T11:14:18

Why are you returning a char * anyway? Why dont you return an SV? Something like this untested version of your code. Also doing *foo++; is pretty misleading since its the same as foo++;.

SV *
getline(str)
        SV * str
        CODE:
        {
                STRLEN len;
                char *cval = SvPV(str, len);
                SV* retsv;
                I32 taken = 0;
                char *ret_str;
                if (!SvOK(str)) {
                        XSRETURN_UNDEF;
                }
                retsv= newSVpvn("",0);
                SvGROW(retsv,len+1);
                ret_str = SvPV_nolen(retsv);
                if (SvTYPE(str) SVt_PVIV)
                        sv_upgrade(str,SVt_PVIV);
                RETVAL = retsv;
                while (*cval) {
                        *ret = *cval;
                        if (*cval == '\n')
                                break;
                        cval++; ret_str++; taken++;
                }
                if (*cval == '\n') {
                        cval++; ret_str++; taken++;
                }
                *ret_str = 0;
                SvCUR_set(retsv,taken);
                if (!SvOOK(str)) {
                        SvOOK_on(str);
                        SvIVX(str) = 0;
                }
                SvIVX(str) += taken;
                SvPVX(str) += taken;
                SvLEN(str) -= taken;
                SvCUR(str) -= taken;
        }
        OUTPUT:
            RETVAL

Re: XS/guts Expert help needed

Matts on 2006-11-11T16:54:21

Thanks, that works perfectly (aside from a minor typo in the main loop) :-)

Re: XS/guts Expert help needed

Kaffeemaschine on 2006-11-12T13:18:10

The char* typemap uses newSVpv to turn the char* into an SV*. newSVpv copies the string into the scalar. Two options: use a CLEANUP section to free the string or use a custom typemap that frees the string.

CLEANUP section: ...
    CLEANUP:
        free (ret);

Custom typemap: in a typemap file:

TYPEMAP
char_own * T_CHAR_OWN
OUTPUT
T_CHAR_OWN
                sv_setpv ((SV*)$arg, $var);
                g_free ($var);

And then use char_own* instead of char* for the type of the return value of the xsub.

Re: XS/guts Expert help needed

demerphq on 2006-11-11T00:41:26

If you did a SAVEFREEPV() on the pointer it would be added to the savestack and freed the next time the save stack was cleared.

But you shouldn't use calloc() or other system level memory allocation routines. You should use New() or one of its variants. This is because Perl may be configured to use its own malloc routines and using the system ones at the same time could lead to issues. IOW, it might work fine for you on your box but might go disasterously wrong when built on a Perl with a different configuration.