Re: [perl #43097] [PATCH] print doesn't overwrite $!

[Christoph Bussenius <lists@der-pepe.de>]

> On Aug 18, 2008, at 18:40, Christoph Bussenius via RT wrote:
>> pp_print calls do_print.  If that is successful *and* the file-handle is
>> autoflushing, pp_print calls PerlIO_flush.
>> The success of do_print depends on the handle's error flag, which gets
>> set when a flush fails and will never be cleared again when writing into
>> the buffer; that means the filehandle will never be flushed again (until
>> the buffer fills up).
>> My fix simply clears the error flag before each write.

I've added a new patch below.  Instead of clearing the error flag, it
will now avoid to consider the previous error flag for the return status
of do_print.  This fixes the bug and still keeps the error flag so that
close knows about previous errors.


On Tue, Aug 19, 2008 at 09:45:24AM +0200, Gisle Aas wrote:
> It's common to just use print 
> without testing its outcome and then in the end verify that all prints 
> succeeded by testing if close() is successful.
>
>      open(my $fh, ">", $file) || die "Can't open $file: $!";
>      print $fh $buf;
>      print $fh $buf;
>      ...
>      close($fh) || die "Can't write to $file: $!";
>
> With your patch the first print could fail and then the second could 
> succeed; leading to close() not reporting the first error.

This piece of code is still a bit problematic in my opinion.  In close's
error message, $! is not guaranteed to describe the error that happened.
If an error happens (as you suggest) in the first print, $! will be
something like "no space left on device".  However, in the passage you
marked as "...", $! can end up with any strange error code, and so the
error that gets reported may not have anything to do with writing to the
file.

I think the only way to get this to work properly is to save errno along
with the error flag for every PerlIO handle.

Regards,
Christoph


--- perl-5.10.0/doio.c	2007-12-18 11:47:07.000000000 +0100
+++ perl-5.10.0-debug/doio.c	2008-09-08 12:32:57.325273275 +0200
@@ -1247,7 +1247,7 @@
 	if (len && (PerlIO_write(fp,tmps,len) == 0))
 	    happy = FALSE;
 	Safefree(tmpbuf);
-	return happy ? !PerlIO_error(fp) : FALSE;
+	return happy;
     }
 }
 
--- perl-5.10.0/t/io/print.t	2007-12-18 11:47:08.000000000 +0100
+++ perl-5.10.0-debug/t/io/print.t	2008-08-18 17:49:05.093301791 +0200
@@ -6,10 +6,10 @@
 }
 
 use strict 'vars';
-eval 'use Errno';
+eval 'use Errno; use IO::Handle';
 die $@ if $@ and !$ENV{PERL_CORE_MINITEST};
 
-print "1..21\n";
+print "1..24\n";
 
 my $foo = 'STDOUT';
 print $foo "ok 1\n";
@@ -65,3 +65,31 @@
     map print(+()), ('')x68;
     print "ok 21\n";
 }
+
+# Create a closed pipe to test print's error reporting
+# See bug #43097
+if (exists $IO::Handle::{autoflush}) {
+    if (pipe my $rpipe, my $wpipe) {
+        eval { $SIG{PIPE} = 'IGNORE'; };
+        close $rpipe or die $!;
+        $wpipe->autoflush(1);
+
+        # printing with autoflush to a closed pipe must fail
+        print $wpipe "xy" and print "not ";
+        print "ok 22\n";
+
+        open my $h, '< /nonexistent' and die '/nonexistent exists';
+
+        print $wpipe "xy";
+        print 'not ' if $!{ENOENT};
+        print "ok 23\n";
+
+        # Printing an empty string must not fail
+        print $wpipe '' or print 'not ';
+        print "ok 24\n";
+    } else {
+        print "ok $_ # skipped: cannot pipe ($!)\n" for 22..24;
+    }
+} else {
+    print "ok $_ # skipped: no autoflush\n" for 22..24;
+}

---

[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index][Thread Index][Top&Search][Original]

---