Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

maybe small error in perlxs.pod example #9414

Closed
p5pRT opened this issue Jul 12, 2008 · 7 comments
Closed

maybe small error in perlxs.pod example #9414

p5pRT opened this issue Jul 12, 2008 · 7 comments

Comments

@p5pRT
Copy link

p5pRT commented Jul 12, 2008

Migrated from rt.perl.org#56874 (status was 'resolved')

Searchable as RT56874$

@p5pRT
Copy link
Author

p5pRT commented Jul 12, 2008

From ajrh@ajrh.net

Created by ajrh@ajrh.net

In the perlxs example (patched below), isn't the newmortal'd SV
being memory-leaked when undef is returned?

Inline Patch
--- perlxs.pod.Orig	2008-07-12 13:14:11.000000000 -0400
+++ perlxs.pod	2008-07-12 13:16:16.974788521 -0400
@@ -1092,12 +1092,12 @@
           char *  host
 	PREINIT:
           time_t  timep;
           bool_t x;
         CODE:
-          ST(0) = sv_newmortal();
           if( rpcb_gettime( host, &timep ) ){
+               ST(0) = sv_newmortal();
                sv_setnv( ST(0), (double)timep);
           }
           else{
                ST(0) = &PL_sv_undef;
           }
Perl Info

Flags:
    category=docs
    severity=medium

Site configuration information for perl 5.10.0:

Configured by Debian Project at Sat Jun 21 21:25:02 UTC 2008.

Summary of my perl5 (revision 5 version 10 subversion 0) configuration:
  Platform:
    osname=linux, osvers=2.6.25.7, archname=i486-linux-gnu-thread-multi
    uname='linux ninsei 2.6.25.7 #1 smp preempt fri jun 20 14:17:13 pdt 2008 i686 gnulinux '
    config_args='-Dusethreads -Duselargefiles -Dccflags=-DDEBIAN -Dcccdlflags=-fPIC -Darchname=i486-linux-gnu -Dprefix=/usr -Dprivlib=/usr/share/perl/5.10 -Darchlib=/usr/lib/perl/5.10 -Dvendorprefix=/usr -Dvendorlib=/usr/share/perl5 -Dvendorarch=/usr/lib/perl5 -Dsiteprefix=/usr/local -Dsitelib=/usr/local/share/perl/5.10.0 -Dsitearch=/usr/local/lib/perl/5.10.0 -Dman1dir=/usr/share/man/man1 -Dman3dir=/usr/share/man/man3 -Dsiteman1dir=/usr/local/man/man1 -Dsiteman3dir=/usr/local/man/man3 -Dman1ext=1 -Dman3ext=3perl -Dpager=/usr/bin/sensible-pager -Uafs -Ud_csh -Ud_ualarm -Uusesfio -Uusenm -DDEBUGGING=-g -Doptimize=-O2 -Duseshrplib -Dlibperl=libperl.so.5.10.0 -Dd_dosuid -des'
    hint=recommended, useposix=true, d_sigaction=define
    useithreads=define, usemultiplicity=define
    useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
    use64bitint=undef, use64bitall=undef, uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='cc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fno-strict-aliasing -pipe -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
    optimize='-O2 -g',
    cppflags='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fno-strict-aliasing -pipe -I/usr/local/include'
    ccversion='', gccversion='4.3.1', gccosandvers=''
    intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234
    d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12
    ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
    alignbytes=4, prototype=define
  Linker and Libraries:
    ld='cc', ldflags =' -L/usr/local/lib'
    libpth=/usr/local/lib /lib /usr/lib /usr/lib64
    libs=-lgdbm -lgdbm_compat -ldb -ldl -lm -lpthread -lc -lcrypt
    perllibs=-ldl -lm -lpthread -lc -lcrypt
    libc=/lib/libc-2.7.so, so=so, useshrplib=true, libperl=libperl.so.5.10.0
    gnulibc_version='2.7'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
    cccdlflags='-fPIC', lddlflags='-shared -O2 -g -L/usr/local/lib'

Locally applied patches:
    


@INC for perl 5.10.0:
    /home/ajrh/lib/perl
    /home/work/lib/perl
    /home/work/lib/perl/linux-debian-x32
    /apps/dam/conf/ajrh/linux-debian-x32/perl
    /etc/perl
    /usr/local/lib/perl/5.10.0
    /usr/local/share/perl/5.10.0
    /usr/lib/perl5
    /usr/share/perl5
    /usr/lib/perl/5.10
    /usr/share/perl/5.10
    /usr/local/lib/site_perl
    .


Environment for perl 5.10.0:
    HOME=/home/ajrh
    LANG=en_US.UTF-8
    LANGUAGE (unset)
    LD_LIBRARY_PATH=/apps/dam/conf/ajrh/linux-debian-x32/lib64:/apps/dam/conf/ajrh/linux-debian-x32/lib32
    LOGDIR (unset)
    PATH=.:/home/ajrh/bin:/home/ajrh/bin/linux-debian-x32:/home/work/fia/scripts:/home/work/scripts:/apps/dam/conf/ajrh/linux-debian-x32/bin:/apps/dam/bin/linux-debian-x32:/usr/local/bin:/usr/bin:/usr/sbin:/bin:/sbin
    PERLLIB=/home/ajrh/lib/perl:/home/work/lib/perl:/home/work/lib/perl/linux-debian-x32:/apps/dam/conf/ajrh/linux-debian-x32/perl
    PERL_BADLANG (unset)
    SHELL=/bin/zsh

@p5pRT
Copy link
Author

p5pRT commented Jul 13, 2008

From @mhx

The newmortal() SV isn't leaked. That's simply because it's "mortal",
and thus it will be freed if its refcount isn't incremented after the
XSUB returns.

But you're right in that the example is suboptimal. The mortal SV
is only needed if it's actually being used afterwards. So I applied
your patch as change #34137.

Marcus

@p5pRT
Copy link
Author

p5pRT commented Jul 13, 2008

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Author

p5pRT commented Jul 13, 2008

@mhx - Status changed from 'open' to 'resolved'

@p5pRT p5pRT closed this as completed Jul 13, 2008
@p5pRT
Copy link
Author

p5pRT commented Jul 13, 2008

From @mhx

The newmortal() SV isn't leaked. That's simply because it's "mortal",
and thus it will be freed if its refcount isn't incremented after the
XSUB returns.

But you're right in that the example is suboptimal. The mortal SV
is only needed if it's actually being used afterwards. So I applied
your patch as change #34137.

Marcus

@p5pRT
Copy link
Author

p5pRT commented Jul 14, 2008

From anthony@ajrh.net

Marcus Holland-Moritz via RT wrote​:

The newmortal() SV isn't leaked. That's simply because it's "mortal",
and thus it will be freed if its refcount isn't incremented after the
XSUB returns.

Are you sure? The mortal SV is no longer accessible via the stack
for cleanup -- the pointer to its address was overwritten.

But you're right in that the example is suboptimal. The mortal SV
is only needed if it's actually being used afterwards. So I applied
your patch as change #34137.

Well, that's fine then regardless. Thanks.

Anthony

@p5pRT
Copy link
Author

p5pRT commented Jul 14, 2008

From anthony@ajrh.net

Anthony Heading wrote​:

Are you sure? The mortal SV is no longer accessible via the stack
for cleanup -- the pointer to its address was overwritten.

Ah - just read perlguts - I hadn't realized there was a separate
stack for temps. My misunderstanding - please ignore the query.

A

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant