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().
--
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