-----BEGIN PGP SIGNED MESSAGE-----
Moin,
On Sunday 11 January 2004 12:48, Nick Ing-Simmons wrote:
> Tels <[EMAIL PROTECTED]> writes:
> >-----BEGIN PGP SIGNED MESSAGE-----
> >
> >##########################################################################
> >#### # DESTROY() - free memory of a GMP number
> >
> >void
> >DESTROY(n)
> > mpz_t* n
> >
> > PPCODE:
> > mpz_clear(*n);
> > free(n);
>
> If it is in a SvPV then you don't call free().
>
> >Sorry, should have included it. Here is the typemap, too:
> >
> >mpz_t * MPZ
> >
> >INPUT
> >MPZ
> > if (sv_derived_from($arg, \"Math::BigInt::GMP\")) {
> > IV tmp = SvIV((SV*)SvRV($arg));
> > $var = ($type) tmp;
> > }
> > else
> > croak(\"$var is not of type Math::BigInt::GMP\")
> >
> >OUTPUT
> >MPZ
> > sv_setref_pv($arg, \"Math::BigInt::GMP\", (void*)$var);
>
> The typemap is wrong. The OUTPUT side puts thing in SvPV the input side
> gets it from SvIV.
I copied this straight from Math::GMP, and for these types of routines it
seems to work ok:
Creating one:
##############################################################################
# _one()
mpz_t *
_one(class)
SV* class
CODE:
NEW_GMP_MPZ_T;
mpz_init_set_ui(*RETVAL, 1);
OUTPUT:
RETVAL
Modyfing one:
##############################################################################
# _add()
mpz_t *
_add(class,x,y)
SV* class
mpz_t * x
mpz_t * y
CODE:
NEW_GMP_MPZ_T_INIT;
mpz_add(*RETVAL, *x, *y);
mpz_set(*x, *RETVAL);
OUTPUT:
RETVAL
Running this:
% cat add.pl
#!/usr/bin/perl -w
use Math::BigInt::GMP;
my $x = Math::BigInt::GMP->_new(\"12");
my $y = Math::BigInt::GMP->_new(\"367");
my $z = Math::BigInt::GMP->_add($x,$y);
print "$x + $y\n = $z\n";
# valgrind perl -Ilib -Iblib/arch add.pl
==15630== Memcheck, a.k.a. Valgrind, a memory error detector for x86-linux.
==15630== Copyright (C) 2002-2003, and GNU GPL'd, by Julian Seward.
==15630== Using valgrind-20031012, a program supervision framework for
x86-linux.
==15630== Copyright (C) 2000-2003, and GNU GPL'd, by Julian Seward.
==15630== Estimated CPU clock rate is 2013 MHz
==15630== For more details, rerun with: -v
==15630==
Math::BigInt::GMP=SCALAR(0x412d7274) + Math::BigInt::GMP=SCALAR(0x412d728c)
= Math::BigInt::GMP=SCALAR(0x412d72d4)
==15630==
==15630== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 4 from 1)
==15630== malloc/free: in use at exit: 400771 bytes in 8064 blocks.
==15630== malloc/free: 15771 allocs, 7707 frees, 12290269 bytes allocated.
==15630== For a detailed leak analysis, rerun with: --leak-check=yes
==15630== For counts of detected errors, rerun with: -v
Gives no indication of errors. So, the code can't be that wrong :)
(Of course, I really have no idea about this, so you are probably right.)
> Which is right depends on how mpz_t's are allocated.
> If they are malloc()ed then SvIV style is correct and you can use DESTROY
> as above with free().
Here is how a mpz_t is allocated:
RETVAL = malloc (sizeof(mpz_t));
mpz_init(*RETVAL);
So mpz_t is just a pointer.
> But _something_ needs to make sure only one SV
> owns the mpz_t * and so free() only happens once.
I guess that is the problem, but I am not sure where to fix this actually. The
"automatic" setup/free via the typemap works, it is just the one routine with
my PUSHs that makes problems.
> Alternatively if mpz_t is stored as opaque binary data in string part of
> SV (SvPV) then OUTPUT still isn't right as binary data will respond badly
> to being strcpy() to the PV, and INPUT is wrong as it is looking in
> wrong SV slot. Also in this case you don't need to free() as SvPV
> will be Safefree()'d by perl. This scheme works quite well with
> perl's SvREFCNT keeping track of when data can go.
I don't think mpz_t is stored as opaque binary data in a string part, but it
could well be that my PUSHs stuff tries to do this (and thusly blows).
Your isight is much appreciated!
>
> >> > mpz_abs (*x, *x);
> >> > PUSHs(sv_setref_pv(sv_newmortal(), "Math::BigInt::GMP",
> >> > (void*)x));
> >>
> >> That is probably wrong. A pv is a char * and perl is going to do
> >> strlen() on it and strcpy() it into memory allocated by perl's New().
> >> My guess is x is already malloc()'ed or C++ new'ed by mpz stuff.
> >> In such cases it is usual to use sv_setref_iv(...,PTR2IV(x)),
> >> and then have DESTROY function call the inner library's free() routine.
But shouldn't the typemap do something equally, instead of doing what it is
doing now? Why does it work with the typemap anyway?
> >The typemap should do something like that, so I should be able just to
> > copy it's code, right?
>
> You could copy its code if its code was correct.
> But to match input it should be
> sv_setref_iv(...,PTR2IV(x))
I'll try that and see if this fixes the problem and doesn't break anything
else.
> > sv_setref_pv($arg, \"Math::BigInt::GMP\", (void*)$var);
> >
> >Looks the same to me. (But the typemaps INPUT looks funny to my naive
> > eyes..)
> >
> >> The other issue you _may_ be having is that *x came in from an SV
> >> (possibly tempoary). Unless *x is refcounted in mpz library you need
> >> get library to give you a new one to return from the XSUB to another
> >> perl SV, or change perl view of API to mimic the library's
> >> modify-in-place semantics. Otherwise both SVs will call DESTROY and try
> >> and free(x).
There is a difference between:
RETVAL = malloc (sizeof(mpz_t));
mpz_init(*RETVAL);
mpz_add( *RETVAL, *x, *y);
(makes a new one, called RETVAL)
and
mpz_add( *x, *x, *y);
(keep x). In my case, both variants crashed with the same error.
Thank you for your time, I have got now some ideas on how to tackle the
problem.
Cheers,
Tels
- --
Signed on Sun Jan 11 13:26:29 2004 with key 0x93B84C15.
Visit my photo gallery at http://bloodgate.com/photos/
PGP key on http://bloodgate.com/tels.asc or per email.
"F�r eine solche Bitratenreduktion muss ich den Transcoder so
umkonfigurieren, dass er gr�bere Quantisierungskoeffizienten f�r die
MPEG-Matrizen verwendet, Captain" - "An die Arbeit, Mr. LaForge." -- Jens
Baumeister in http://tinyurl.com/oomb
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.2-rc1-SuSE (GNU/Linux)
Comment: When cryptography is outlawed, bayl bhgynjf jvyy unir cevinpl.
iQEVAwUBQAFDincLPEOTuEwVAQGWXgf/TFuRHqCM1Qecmf65vNc4KMl+A8qxGgWZ
2HBTL2VAYyoZ5GaNDtxUnsLCiNnFKmGO9ltutEMbbwWIus6Qv5eCnnOTUd2ce5z7
5TUfelEKwPlK2wLcwkA1KSbRWK4miaz92DZ7HZE+SEPlqL/dHkE7HOWMtzY71LFK
aqpZn92d+yC18JdDW+z0yXrbFrSConF9l9xsjSGoNXd1/zA4pl5eiZQneEcXUX2s
OPGiVsFO/BdFLnKppWuK9LCGkukG9Jr77YaU0HYQVgEPHidot0nViYg1LTjU+X/0
AEjHqix195t6Q/aZrE9I6e+yYniLvDYRRhtMyXWdb6gujm9oNtfqKQ==
=h2ar
-----END PGP SIGNATURE-----
