develooper Front page | perl.perl5.porters | Postings from February 2008

Re: Assistance with IPC modules and perl 5.10

Thread Previous | Thread Next
From:
Nicholas Clark
Date:
February 23, 2008 13:10
Subject:
Re: Assistance with IPC modules and perl 5.10
Message ID:
20080223210847.GR38653@plum.flirble.org
On Sat, Feb 23, 2008 at 05:44:34PM +0000, Nicholas Clark wrote:
> On Fri, Feb 15, 2008 at 06:44:09PM -0500, Tom Callaway wrote:
> 
> > You can see the build log (and test failures) for IPC::ShareLite here:
> > http://koji.fedoraproject.org/koji/getfile?taskID=424041&name=build.log
> > 
> > (note that IPC::ShareLite is patched with
> > http://rt.cpan.org/Public/Bug/Display.html?id=17663, normally it just
> > segfaults during the tests instead of failing)

I assume that the source you build from is

http://download.fedora.redhat.com/pub/fedora/linux/development/source/SRPMS/perl-IPC-ShareLite-0.09-12.fc9.src.rpm

> Moreover, your log shows this:
> 
> ShareLite.c: In function 'XS_IPC__ShareLite_read_share':
> ShareLite.xs:338: warning: 'length' is used uninitialized in this function
> ShareLite.xs:336: note: 'length' was declared here
> ShareLite.xs:339: warning: 'data' is used uninitialized in this function
> ShareLite.xs:335: note: 'data' was declared here
> 
> Which seems very very wrong, but I can't get a C compiler to scream about that.

The patch on rt.cpan.org is this:

diff -u IPC-ShareLite-0.09/ShareLite.xs IPC-ShareLite-0.09_fixed/
ShareLite.xs
--- IPC-ShareLite-0.09/ShareLite.xs     Sat Jul 15 10:59:52 2000
+++ IPC-ShareLite-0.09_fixed/ShareLite.xs       Wed Feb 15 18:30:08 2006
@@ -335,12 +335,17 @@
     char*    data;
     int      length;
   CODE:
-    share  = (Share *)SvIV(ST(0));
     length = read_share(share, &data);
-    ST(0) = sv_newmortal();
-    if (length >= 0) {
-      sv_usepvn((SV*)ST(0), data, length);
-    } else {
+    if (length > 0) {
+      ST(0) = sv_2mortal(newSVpv(data, length));
+      free(data);
+    }
+    else if (length == 0) {
+      ST(0) = sv_2mortal(newSVpv("", 0));
+      free(data); /* XXX? */
+    }
+    else {
+      ST(0) = sv_newmortal();
       sv_setsv(ST(0), &PL_sv_undef);
     }

The patch in the SRPM is this:

diff -up IPC-ShareLite-0.09/ShareLite.xs.BAD IPC-ShareLite-0.09/ShareLite.xs
--- IPC-ShareLite-0.09/ShareLite.xs.BAD	2008-02-09 16:54:47.000000000 -0500
+++ IPC-ShareLite-0.09/ShareLite.xs	2008-02-09 16:57:21.000000000 -0500
@@ -335,12 +335,16 @@ read_share(share)
     char*    data; 
     int      length;
   CODE:
-    share  = (Share *)SvIV(ST(0));
-    length = read_share(share, &data);
-    ST(0) = sv_newmortal();
-    if (length >= 0) {
-      sv_usepvn((SV*)ST(0), data, length);
-    } else {
+    if (length > 0) {
+      ST(0) = sv_2mortal(newSVpv(data, length));
+      free(data);
+    }
+    else if (length == 0) {
+      ST(0) = sv_2mortal(newSVpv("", 0));
+      free(data); /* XXX? */
+    }
+    else {
+      ST(0) = sv_newmortal();
       sv_setsv(ST(0), &PL_sv_undef);
     }


Which is not the same!

Specifically, the SRPM patch removes the call to read_share(). Oops. :-)

I suspect that this is the cause of the problem, and hope that fixing just it
will make everything peachy.

However, the appended patch saves one round of copying, and I think will work
on any configuration (threaded/not, mymalloc/not, -DDEBUGGING'schecking/not)
However, not tested on all permutations, and not tested on anything big endian.

It all reminds me of this bug of mine:

http://www.nntp.perl.org/group/perl.perl5.changes/2006/02/msg14968.html

Nicholas Clark
Thread Previous | Thread Next


nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About