develooper Front page | perl.perl5.porters | Postings from March 2012

Re: [perl #111126] perlbug AutoReply: File::Spec copy() zeros filewhen copying a file into the same directory it is stored in.

From:
David Yingling
Date:
March 29, 2012 02:05
Subject:
Re: [perl #111126] perlbug AutoReply: File::Spec copy() zeros filewhen copying a file into the same directory it is stored in.
Message ID:
20120329001651.0582d0f0.deeelwy@gmail.com
On Fri, 09 Mar 2012 00:51:48 -0800
"Tony Cook via RT" <perlbug-followup@perl.org> wrote:

> On Wed, Feb 22, 2012 at 02:36:41AM -0500, David Yingling wrote:
> > Because of this change the code that comes next that calls carp() with an error message actually does run properly, and carp() is called with an error message, and copy() returns 0 for failure.
> 
> Actually, I was thinking the user might expect the error message to
> use the same input and output names that the user specified.

Ok, I understand what you mean now, because _catname() changes the $to variable by adding the basename/filename of $from to it, so the error message will report a different value of $to than the user provides to the copy() function.

> 
> This is a new failure case, so the caller can't really have any
> expectations.
> 

Yeah, I suppose you could copy $to and $from into say $orig_to and $orig_from at the very top before anything changes $to and $from. And then replace all occurrence's of $to and $from in error messages to $orig_to and $orig_from.  Like in the patch below.

--- lib/File/OrigCopy.pm        2012-03-28 23:58:17.507741191 -0400
+++ lib/File/Copy.pm    2012-03-28 23:54:36.872741163 -0400
@@ -121,6 +121,8 @@
     my $from = shift;
     my $to = shift;
 
+    my ($orig_from, $orig_to) = ($from, $to);
+
     my $size;
     if (@_) {
        $size = shift(@_) + 0;
@@ -139,7 +141,7 @@
                         : (ref(\$to) eq 'GLOB'));
 
     if (_eq($from, $to)) { # works for references, too
-       carp("'$from' and '$to' are identical (not copied)");
+       carp("'$orig_from' and '$orig_to' are identical (not copied)");
         # The "copy" was a success as the source and destination contain
         # the same data.
         return 1;
@@ -155,7 +157,7 @@
        if (@fs) {
            my @ts = stat($to);
            if (@ts && $fs[0] == $ts[0] && $fs[1] == $ts[1] && !-p $from) {
-               carp("'$from' and '$to' are identical (not copied)");
+               carp("'$orig_from' and '$orig_to' are identical (not copied)");
                 return 0;
            }
        }

But is this change just for a less confusing error messages needed? Will some one come across this behavior in the future, and submit a bug report, because copy() renamed their arguments in an error message?

Also, nothing changes $from, but my patch has an unnecessary $orig_from in it. That could be removed. (I noticed it after I made the patch.)

I don't know. I'll leave the decision for this up to you.

> Attached, patches introducing a TODO test for the desired behaviour
> and the change, unTODOing the test.
> 
> I intend to apply this post 5.16, unless the pumpkin thinks it's worth
> applying before then.
>

Ok, Thanks,
Dave.
 
> Tony
> 



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