Subject: Re: [PATCH v3 2/4] archive-tar: write extended headers for file sizes >= 8GB

From: Junio C Hamano <[email-masked]>
Date: Fri, 24 Jun 2016 12:01:16 -0700

Jeff King <[email-masked]> writes:

> If the size was 64GB or greater, then we actually overflowed
> digits into the mtime field, meaning our value was was

was was

> effectively right-shifted by those lost octal digits. And
> this patch is again a strict improvement over that.

> diff --git a/archive-tar.c b/archive-tar.c
> index cb99df2..274bdfa 100644
> --- a/archive-tar.c
> +++ b/archive-tar.c
> @@ -137,6 +137,20 @@ static void strbuf_append_ext_header(struct strbuf *sb, const char *keyword,
>  	strbuf_addch(sb, '\n');
>  }
>  
> +/*
> + * Like strbuf_append_ext_header, but for numeric values.
> + */
> +static void strbuf_append_ext_header_uint(struct strbuf *sb,
> +					  const char *keyword,
> +					  uintmax_t value)
> +{
> +	char buf[40]; /* big enough for 2^128 in decimal, plus NUL */
> +	int len;
> +
> +	len = xsnprintf(buf, sizeof(buf), "%"PRIuMAX, value);
> +	strbuf_append_ext_header(sb, keyword, buf, len);
> +}
> +
>  static unsigned int ustar_header_chksum(const struct ustar_header *header)
>  {
>  	const unsigned char *p = (const unsigned char *)header;
> @@ -208,7 +222,7 @@ static int write_tar_entry(struct archiver_args *args,
>  	struct ustar_header header;
>  	struct strbuf ext_header = STRBUF_INIT;
>  	unsigned int old_mode = mode;
> -	unsigned long size;
> +	unsigned long size, size_in_header;
>  	void *buffer;
>  	int err = 0;
>  
> @@ -267,7 +281,13 @@ static int write_tar_entry(struct archiver_args *args,
>  			memcpy(header.linkname, buffer, size);
>  	}
>  
> -	prepare_header(args, &header, mode, size);
> +	size_in_header = size;
> +	if (S_ISREG(mode) && size > 077777777777UL) {

Want a symbolic constant with a comment that says why you have
eleven 7's?

> +		size_in_header = 0;
> +		strbuf_append_ext_header_uint(&ext_header, "size", size);
> +	}
> +
> +	prepare_header(args, &header, mode, size_in_header);

The change itself makes sense.

Thanks.