threads / patch / 45864

patcharchive-tar: fix a sparse 'constant too large' warning

Subject: [PATCH] archive-tar: fix a sparse 'constant too large' warning

## tl;dr

4 messages between May 3, 2017 and May 8, 2017. Diffs are folded; open one to read it.

replies: 3people: 3as markdown or json

Ramsay Jones· May 3, 2017, 23:41 UTC · lore

Commit dddbad728c ("timestamp_t: a new data type for timestamps", 26-04-2017) introduced a new typedef 'timestamp_t', as a synonym for an unsigned long, which was used at the time to represent timestamps in git. A later commit 28f4aee3fb ("use uintmax_t for timestamps", 26-04-2017) changed the typedef to use an 'uintmax_t' for the timestamp representation type.

When building on a 32-bit Linux system, sparse complains that a constant (USTAR_MAX_MTIME) used to detect a 'far-future mtime' timestamp, is too large; 'warning: constant 077777777777UL is so big it is unsigned long long' on lines 335 and 338 of archive-tar.c. Note that both gcc and clang only issue a warning if this constant is used in a context that requires an 'unsigned long' (rather than an uintmax_t). (Since TIME_MAX is no longer equal to 0xFFFFFFFF, even on a 32-bit system, the macro USTAR_MAX_MTIME is set to 077777777777UL, which cannot be represented as an 'unsigned long' constant).

In order to suppress the warning, change the definition of the macro constant to use an 'ULL' type suffix.

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---

Hi Junio,

I try to build git on 32-bit Linux about once a week (but it can sometimes be less often than that!). During tonight's build sparse chirped at me while building the 'next' branch.

This patch was built on top of next.

ATB, Ramsay Jones

 archive-tar.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Show changes to archive-tar.c +1 −1

diff --git a/archive-tar.c b/archive-tar.c
index 319a5b1c7..6dddc0cff 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -33,7 +33,7 @@ static int write_tar_filter_archive(const struct archiver *ar,
 #if TIME_MAX == 0xFFFFFFFF
 #define USTAR_MAX_MTIME TIME_MAX
 #else
-#define USTAR_MAX_MTIME 077777777777UL
+#define USTAR_MAX_MTIME 077777777777ULL
 #endif
 
 /* writes out the whole block, but only if it is full */

-- 
2.12.0

Johannes Schindelin· May 4, 2017, 09:26 UTC · re: Ramsay Jones · lore

Re: [PATCH] archive-tar: fix a sparse 'constant too large' warning

Hi Ramsay,

On Thu, 4 May 2017, Ramsay Jones wrote:

Show 12 quoted lines

> diff --git a/archive-tar.c b/archive-tar.c
> index 319a5b1c7..6dddc0cff 100644
> --- a/archive-tar.c
> +++ b/archive-tar.c
> @@ -33,7 +33,7 @@ static int write_tar_filter_archive(const struct archiver *ar,
>  #if TIME_MAX == 0xFFFFFFFF
>  #define USTAR_MAX_MTIME TIME_MAX
>  #else
> -#define USTAR_MAX_MTIME 077777777777UL
> +#define USTAR_MAX_MTIME 077777777777ULL
>  #endif
>

Funny. This problem was pointed out by Hannes Sixt (IIRC) and I fixed this very thing in v6.

Except I did not. I changed the wrong constant! Instead of USTAR_MAX_MTIME, I adjusted USTAR_MAX_SIZE. D'oh.

I just saw that my patch series already hit `next`, so I fear you are right that we need a follow-up patch. Maybe we want this diff, though?

-- snipsnap --

Show changes to archive-tar.c +2 −2

diff --git a/archive-tar.c b/archive-tar.c
index 319a5b1c7cd..073e60ebd3c 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -28,12 +28,12 @@ static int write_tar_filter_archive(const struct archiver *ar,
 #if ULONG_MAX == 0xFFFFFFFF
 #define USTAR_MAX_SIZE ULONG_MAX
 #else
-#define USTAR_MAX_SIZE 077777777777ULL
+#define USTAR_MAX_SIZE 077777777777UL
 #endif
 #if TIME_MAX == 0xFFFFFFFF
 #define USTAR_MAX_MTIME TIME_MAX
 #else
-#define USTAR_MAX_MTIME 077777777777UL
+#define USTAR_MAX_MTIME 077777777777ULL
 #endif
 
 /* writes out the whole block, but only if it is full */

Ramsay Jones· May 5, 2017, 01:21 UTC · re: Johannes Schindelin · lore

Re: [PATCH] archive-tar: fix a sparse 'constant too large' warning

On 04/05/17 10:26, Johannes Schindelin wrote:

Show 22 quoted lines

> Hi Ramsay,
> 
> On Thu, 4 May 2017, Ramsay Jones wrote:
> 
>> diff --git a/archive-tar.c b/archive-tar.c
>> index 319a5b1c7..6dddc0cff 100644
>> --- a/archive-tar.c
>> +++ b/archive-tar.c
>> @@ -33,7 +33,7 @@ static int write_tar_filter_archive(const struct archiver *ar,
>>  #if TIME_MAX == 0xFFFFFFFF
>>  #define USTAR_MAX_MTIME TIME_MAX
>>  #else
>> -#define USTAR_MAX_MTIME 077777777777UL
>> +#define USTAR_MAX_MTIME 077777777777ULL
>>  #endif
>>  
> 
> Funny. This problem was pointed out by Hannes Sixt (IIRC) and I fixed this
> very thing in v6.
> 
> Except I did not. I changed the wrong constant! Instead of
> USTAR_MAX_MTIME, I adjusted USTAR_MAX_SIZE. D'oh.

Ah, I did wonder about that.

Show 14 quoted lines

> I just saw that my patch series already hit `next`, so I fear you are
> right that we need a follow-up patch. Maybe we want this diff, though?
> 
> -- snipsnap --
> diff --git a/archive-tar.c b/archive-tar.c
> index 319a5b1c7cd..073e60ebd3c 100644
> --- a/archive-tar.c
> +++ b/archive-tar.c
> @@ -28,12 +28,12 @@ static int write_tar_filter_archive(const struct archiver *ar,
>  #if ULONG_MAX == 0xFFFFFFFF
>  #define USTAR_MAX_SIZE ULONG_MAX
>  #else
> -#define USTAR_MAX_SIZE 077777777777ULL
> +#define USTAR_MAX_SIZE 077777777777UL

Sure, I can add this. (Although, I don't think it actually matters).

Hmm, unless somebody objects in the meantime, I will re-roll the patch (tomorrow, it's late!).

Show 10 quoted lines

>  #endif
>  #if TIME_MAX == 0xFFFFFFFF
>  #define USTAR_MAX_MTIME TIME_MAX
>  #else
> -#define USTAR_MAX_MTIME 077777777777UL
> +#define USTAR_MAX_MTIME 077777777777ULL
>  #endif
>  
>  /* writes out the whole block, but only if it is full */
>

ATB, Ramsay Jones

Junio C Hamano· May 8, 2017, 00:19 UTC · re: Ramsay Jones · lore

Re: [PATCH] archive-tar: fix a sparse 'constant too large' warning

Ramsay Jones <ramsay@ramsayjones.plus.com> writes:

Show 5 quoted lines

> Sure, I can add this. (Although, I don't think it actually
> matters).
>
> Hmm, unless somebody objects in the meantime, I will re-roll
> the patch (tomorrow, it's late!).

Thanks.