on Thu Jan 29 2009, Jeff King <peff-AT-peff.net> wrote:
On Thu, Jan 29, 2009 at 12:20:41AM -0500, Jeff King wrote:
quoted
Ok, that _is_ big. ;) I wouldn't be surprised if there is some corner of
the code that barfs on a single object that doesn't fit in a signed
32-bit integer; I don't think we have any test coverage for stuff that
big.
Sure enough, that is the problem. With the patch below I was able to
"git add" and commit a 3 gigabyte file of random bytes (so even the
deflated object was 3G).
I think it might be worth applying as a general cleanup, but I have no
idea if other parts of the system might barf on such an object.
-- >8 --
Subject: [PATCH] avoid 31-bit truncation in write_loose_object
The size of the content we are adding may be larger than
2.1G (i.e., "git add gigantic-file"). Most of the code-path
to do so uses size_t or unsigned long to record the size,
but write_loose_object uses a signed int.
On platforms where "int" is 32-bits (which includes x86_64
Linux platforms), we end up passing malloc a negative size.
Good work. I don't know if this matters to you, but I think on a 32-bit
platform you'll find that size_t, which is supposed to be able to hold
the size of the largest representable *memory block*, is only 4 bytes
large:
#include <limits.h>
#include <stdio.h>
int main()
{
printf("sizeof(size_t) = %d", sizeof(size_t));
}
Prints "sizeof(size_t) = 4" on my core duo.
quoted hunk
Signed-off-by: Jeff King <redacted>
---
sha1_file.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index 360f7e5..8868b80 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2340,7 +2340,8 @@ static int create_tmpfile(char *buffer, size_t bufsiz, const
char *filename)
static int write_loose_object(const unsigned char *sha1, char *hdr, int hdrlen,
void *buf, unsigned long len, time_t mtime)
{
- int fd, size, ret;
+ int fd, ret;
+ size_t size;
unsigned char *compressed;
z_stream stream;
char *filename;
--
Dave Abrahams
BoostPro Computing
http://www.boostpro.com
David Abrahams wrote:
on Thu Jan 29 2009, Jeff King <peff-AT-peff.net> wrote:
quoted
On Thu, Jan 29, 2009 at 12:20:41AM -0500, Jeff King wrote:
quoted
Ok, that _is_ big. ;) I wouldn't be surprised if there is some corner of
the code that barfs on a single object that doesn't fit in a signed
32-bit integer; I don't think we have any test coverage for stuff that
big.
Sure enough, that is the problem. With the patch below I was able to
"git add" and commit a 3 gigabyte file of random bytes (so even the
deflated object was 3G).
I think it might be worth applying as a general cleanup, but I have no
idea if other parts of the system might barf on such an object.
-- >8 --
Subject: [PATCH] avoid 31-bit truncation in write_loose_object
The size of the content we are adding may be larger than
2.1G (i.e., "git add gigantic-file"). Most of the code-path
to do so uses size_t or unsigned long to record the size,
but write_loose_object uses a signed int.
On platforms where "int" is 32-bits (which includes x86_64
Linux platforms), we end up passing malloc a negative size.
Good work. I don't know if this matters to you, but I think on a 32-bit
platform you'll find that size_t, which is supposed to be able to hold
the size of the largest representable *memory block*, is only 4 bytes
large:
#include <limits.h>
#include <stdio.h>
int main()
{
printf("sizeof(size_t) = %d", sizeof(size_t));
}
Prints "sizeof(size_t) = 4" on my core duo.
It has nothing to do with typesize, and everything to do with
signedness. A size_t cannot be negative, while an int can.
Making sure we use the correct signedness everywhere means
we double the capacity where negative values are clearly bogus,
such as in this case. On 32-bit platforms, the upper limit for
what git can handle is now 4GB, which is expected. To go beyond
that, we'd need to rework the algorithm so we handle chunks of
the data instead of the whole. Some day, that might turn out to
be necessary but today is not that day.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
On Thu, Jan 29, 2009 at 08:10:05AM -0500, David Abrahams wrote:
Good work. I don't know if this matters to you, but I think on a 32-bit
platform you'll find that size_t, which is supposed to be able to hold
the size of the largest representable *memory block*, is only 4 bytes
large:
That should be fine; 32-bit systems can't deal with such large files
anyway, since we want to address the whole thing. Getting around that
would, as Andreas mentioned, involve dealing with large files in chunks,
something that would make the code a lot more complex.
So I think the answer is "tough, if you want files >4G get a 64-bit
machine". Which is unreasonable for a file system to say, but I think is
fine for git.
-Peff