Re: [PATCH v4 3/5] archive-tar: write extended headers for file sizes >= 8GB

From: Torsten Bögershausen <tboegi@web.de>
Date: Jul 15, 2016, 02:59 UTC
Message-ID: <7ad7705a-40a1-cc2c-e7ea-47d83e1627a3@web.de>
In-Reply-To: <ac016bb4-9f69-7e03-bc99-7a5bbc1a1a33@kdbg.org>

On 07/14/2016 06:48 PM, Johannes Sixt wrote:

Show 23 quoted lines

> Am 30.06.2016 um 11:09 schrieb Jeff King:
>> +/*
>> + * This is the max value that a ustar size header can specify, as it
>> is fixed
>> + * at 11 octal digits. POSIX specifies that we switch to extended
>> headers at
>> + * this size.
>> + */
>> +#define USTAR_MAX_SIZE 077777777777UL
>
> This is too large by one bit for our 32-bit unsigned long on Windows:
>
> archive-tar.c: In function 'write_tar_entry':
> archive-tar.c:295: warning: integer constant is too large for 'unsigned
> long' type
> archive-tar.c: In function 'write_global_extended_header':
> archive-tar.c:332: warning: integer constant is too large for 'unsigned
> long' type
> archive-tar.c:335: warning: integer constant is too large for 'unsigned
> long' type
> archive-tar.c:335: warning: overflow in implicit constant conversion
>
> -- Hannes

Similar problem on 32 Bit Linux:
  In function ‘write_global_extended_header’:
archive-tar.c:29:25: warning: overflow in implicit constant conversion 
[-Woverflow]
  #define USTAR_MAX_MTIME 077777777777UL
                          ^
archive-tar.c:335:16: note: in expansion of macro ‘USTAR_MAX_MTIME’
    args->time = USTAR_MAX_MTIME;

---------------------------------- I want to volunteer to do more tests on 32 bit Linux ;-) Does this fix it and look as a good thing to change ?

--- a/archive-tar.c
+++ b/archive-tar.c
@@ -332,7 +332,7 @@ static void write_global_extended_header(struct 
archiver_args *args)
         if (args->time > USTAR_MAX_MTIME) {
                 strbuf_append_ext_header_uint(&ext_header, "mtime",
                                               args->time);
-               args->time = USTAR_MAX_MTIME;
+               args->time = (time_t)USTAR_MAX_MTIME;
         }