Re: [PATCH 3/5] archive: delegate blob reading to backend

["René Scharfe" <rene.scharfe@lsrfire.ath.cx>]

Am 30.04.2012 06:57, schrieb Nguyễn Thái Ngọc Duy:
> archive-tar.c and archive-zip.c now perform conversion check, with
> help of sha1_file_to_archive() from archive.c
> 
> This gives backends more freedom in dealing with (streaming) large
> blobs.
> 
> Signed-off-by: Nguyễn Thái Ngọc Duy<pclouds@gmail.com>
> ---
>   archive-tar.c |   20 +++++++++++++++++---
>   archive-zip.c |   14 ++++++++++++--
>   archive.c     |   28 +++++++++++-----------------
>   archive.h     |   10 +++++++++-
>   4 files changed, 49 insertions(+), 23 deletions(-)
> 
> diff --git a/archive-tar.c b/archive-tar.c
> index 6c8a0bd..61821f4 100644
> --- a/archive-tar.c
> +++ b/archive-tar.c
> @@ -161,11 +161,15 @@ static int write_extended_header(struct archiver_args *args,
>   }
> 
>   static int write_tar_entry(struct archiver_args *args,
> -		const unsigned char *sha1, const char *path, size_t pathlen,
> -		unsigned int mode, void *buffer, unsigned long size)
> +			   const unsigned char *sha1,
> +			   const char *path, size_t pathlen,
> +			   unsigned int mode)
>   {
>   	struct ustar_header header;
>   	struct strbuf ext_header = STRBUF_INIT;
> +	unsigned int old_mode = mode;
> +	unsigned long size;
> +	void *buffer;
>   	int err = 0;
> 
>   	memset(&header, 0, sizeof(header));
> @@ -199,7 +203,17 @@ static int write_tar_entry(struct archiver_args *args,
>   	} else
>   		memcpy(header.name, path, pathlen);
> 
> -	if (S_ISLNK(mode)&&  buffer) {
> +	if (S_ISLNK(mode) || S_ISREG(mode)) {
> +		enum object_type type;
> +		buffer = sha1_file_to_archive(args, path, sha1, old_mode,&type,&size);

This buffer is never freed.

> +		if (!buffer)
> +			return error("cannot read %s", sha1_to_hex(sha1));
> +	} else {
> +		buffer = NULL;
> +		size = 0;
> +	}
> +
> +	if (S_ISLNK(mode)) {
>   		if (size>  sizeof(header.linkname)) {
>   			sprintf(header.linkname, "see %s.paxheader",
>   			        sha1_to_hex(sha1));
> diff --git a/archive-zip.c b/archive-zip.c
> index 02d1f37..f8039ba 100644
> --- a/archive-zip.c
> +++ b/archive-zip.c
> @@ -121,8 +121,9 @@ static void *zlib_deflate(void *data, unsigned long size,
>   }
> 
>   static int write_zip_entry(struct archiver_args *args,
> -		const unsigned char *sha1, const char *path, size_t pathlen,
> -		unsigned int mode, void *buffer, unsigned long size)
> +			   const unsigned char *sha1,
> +			   const char *path, size_t pathlen,
> +			   unsigned int mode)
>   {
>   	struct zip_local_header header;
>   	struct zip_dir_header dirent;
> @@ -134,6 +135,8 @@ static int write_zip_entry(struct archiver_args *args,
>   	int method;
>   	unsigned char *out;
>   	void *deflated = NULL;
> +	void *buffer;
> +	unsigned long size;
> 
>   	crc = crc32(0, NULL, 0);
> 
> @@ -148,7 +151,14 @@ static int write_zip_entry(struct archiver_args *args,
>   		out = NULL;
>   		uncompressed_size = 0;
>   		compressed_size = 0;
> +		buffer = NULL;
> +		size = 0;
>   	} else if (S_ISREG(mode) || S_ISLNK(mode)) {
> +		enum object_type type;
> +		buffer = sha1_file_to_archive(args, path, sha1, mode,&type,&size);

This one is leaked as well.

> +		if (!buffer)
> +			return error("cannot read %s", sha1_to_hex(sha1));
> +
>   		method = 0;
>   		attr2 = S_ISLNK(mode) ? ((mode | 0777)<<  16) :
>   			(mode&  0111) ? ((mode)<<  16) : 0;

You could squash this in:


diff --git a/archive-tar.c b/archive-tar.c
index 61821f4..bb320ad 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -235,6 +235,7 @@ static int write_tar_entry(struct archiver_args *args,
 	write_blocked(&header, sizeof(header));
 	if (S_ISREG(mode) && buffer && size > 0)
 		write_blocked(buffer, size);
+	free(buffer);
 	return err;
 }
 
diff --git a/archive-zip.c b/archive-zip.c
index f8039ba..716cc42 100644
--- a/archive-zip.c
+++ b/archive-zip.c
@@ -239,6 +239,7 @@ static int write_zip_entry(struct archiver_args *args,
 	}
 
 	free(deflated);
+	free(buffer);
 
 	return 0;
 }