Re: [PATCH 4/5] archive-tar: stream large blobs to tar file

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 30, 2012, 19:01 UTC
Message-ID: <7vmx5tf3h6.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1335761837-12482-5-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

Show 23 quoted lines

> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  archive-tar.c    |   38 +++++++++++++++++++++++++++++++++++---
>  t/t1050-large.sh |    4 ++++
>  2 files changed, 39 insertions(+), 3 deletions(-)
>
> diff --git a/archive-tar.c b/archive-tar.c
> index 61821f4..865ef6d 100644
> --- a/archive-tar.c
> +++ b/archive-tar.c
> @@ -4,6 +4,7 @@
>  #include "cache.h"
>  #include "tar.h"
>  #include "archive.h"
> +#include "streaming.h"
>  #include "run-command.h"
>  
>  #define RECORDSIZE	(512)
> @@ -62,6 +63,29 @@ static void write_blocked(const void *data, unsigned long size)
>  	write_if_needed();
>  }
>  
> +static int stream_blob_to_file(const unsigned char *sha1)

The name of this function is misleading and caused me to look for an existing implementation that streams blob contents to a given fd while wondering how that fd is passed to this function.

I do not think it would make sense to reuse stream_blob_to_fd() here, as is it too much hassle to account for blocking factors. In order to make it clear for future futzers that this is not something they can turn into global function and reuse outside the context of generating tar archive output, please rename it either to stream_blob_to_tarfile(), which is more specific, or stream_blocked(), which is too vague and nobody sane would think about exporting from the file just like existing write_blocked() is.