Am 30.04.2012 23:36, schrieb Junio C Hamano:
René Scharfe<rene.scharfe@xxxxxxxxxxxxxx> writes:+static int stream_blob_to_file(const unsigned char *sha1) +{ + struct git_istream *st; + enum object_type type; + unsigned long sz; + + st = open_istream(sha1,&type,&sz, NULL); + if (!st) + return error("cannot stream blob %s", sha1_to_hex(sha1)); + for (;;) { + char buf[BLOCKSIZE]; + ssize_t readlen; + + readlen = read_istream(st, buf, sizeof(buf)); + + if (readlen<= 0) + return readlen; + write_blocked(buf, readlen); + } + close_istream(st); + return 0; +}The stream is never closed. Perhaps squash this in? diff --git a/archive-tar.c b/archive-tar.c index 506c8cb..6109fd3 100644 --- a/archive-tar.c +++ b/archive-tar.c @@ -66,6 +66,7 @@ static void write_blocked(const void *data, unsigned long size) static int stream_blob_to_file(const unsigned char *sha1) { struct git_istream *st; + ssize_t readlen; enum object_type type; unsigned long sz; @@ -74,16 +75,15 @@ static int stream_blob_to_file(const unsigned char *sha1) return error("cannot stream blob %s", sha1_to_hex(sha1)); for (;;) { char buf[BLOCKSIZE]; - ssize_t readlen; readlen = read_istream(st, buf, sizeof(buf)); if (readlen<= 0) - return readlen; + break; write_blocked(buf, readlen); } close_istream(st); - return 0; + return readlen; }Your patch on top obviouly is the right thing to do, but reading the code again, I am not sure if the original is correct. read_istream() itself does not promise that it will always fill the buffer before returning (it could return with a short read). It seems incorrect that the caller does not loop to avoid padding a short read with padding by calling write_blocked().
Yes, indeed, good catch. It could also write to block directly and avoid copying the buffer again. The tail clearing part of write_blocked() can certainly be reused, but the rest will probably have to be reimplemented around read_istream().
René -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html