Re: [PATCH] Fix in Git.pm cat_blob crashes on large files (resubmit with reviewed-by)
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:56:13
Joshua Clayton [off-list ref] writes:
Read and write each 1024 byte buffer, rather than trying to buffer the entire content of the file. Previous code would crash on all files > 2 Gib, when the offset variable became negative (perhaps below the level of perl), resulting in a crash. On a 32 bit system, or a system with low memory it might crash before reaching 2 GiB due to memory exhaustion. Signed-off-by: Joshua Clayton <redacted> Reviewed-by: Jeff King <redacted> ---
Thanks.
quoted
Subject: Re: [PATCH] Fix in Git.pm cat_blob crashes on large files (resubmit with reviewed-by)
Please drop the () part. A rule of thumb is to make "git show" output understandable by people who read it 6 months from now. They do not care if the commit is a re-submission. It seems that this issue was with us since the very beginning of this sub since it was introduced at 7182530d8cad (Git.pm: Add hash_and_insert_object and cat_blob, 2008-05-23).
quoted hunk
perl/Git.pm | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-)diff --git a/perl/Git.pm b/perl/Git.pm index 931047c..cc91288 100644 --- a/perl/Git.pm +++ b/perl/Git.pm@@ -949,13 +949,16 @@ sub cat_blob { last unless $bytesLeft; my $bytesToRead = $bytesLeft < 1024 ? $bytesLeft : 1024; - my $read = read($in, $blob, $bytesToRead, $bytesRead); + my $read = read($in, $blob, $bytesToRead); unless (defined($read)) { $self->_close_cat_blob(); throw Error::Simple("in pipe went bad"); } - $bytesRead += $read; + unless (print $fh $blob) { + $self->_close_cat_blob(); + throw Error::Simple("couldn't write to passedin filehandle"); + }
Corrupt patch, line-wrapped by your MUA.
I wonder if we still need $bytesRead variable. You have $size that
is the size of the whole blob, so
my $bytesLeft = $size;
while (1) {
my $bytesToRead = $bytesLeft < 1024 ? $bytesLeft : 1024;
my $bytesRead = read($in, $blob, $bytesToRead);
... check errors and use the $blob ...
$bytesLeft -= $bytesRead;
}
may be simpler and easier to read, no?