git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Ramsay Jones <ramsay@ramsayjones.plus.com>
Date
May 9, 2017, 20:13 UTC
Message-ID
<5068a9b6-c77a-737d-9d80-40108c5f22c3@ramsayjones.plus.com>
In-Reply-To
<alpine.DEB.2.21.1.1705091223190.146734@virtualbox>
On 09/05/17 11:24, Johannes Schindelin wrote:
Show 33 quoted lines
> Hi Ramsay,
> 
> On Mon, 8 May 2017, Ramsay Jones wrote:
> 
>> 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 USTAR_MAX_MTIME to use an 'ULL' type suffix.
>>
>> In a similar vein, on systems which use a 64-bit representation of the
>> 'unsigned long' type, the USTAR_MAX_SIZE constant macro is defined with
>> the value 077777777777ULL. Although this does not cause any warning
>> messages to be issued, it would be more appropriate for this constant
>> to use an 'UL' type suffix rather than 'ULL'.
> 
> 	The reason for the current situation is that an earlier fix for
> 	the USTAR_MAX_MTIME constant was applied to the USTAR_MAX_SIZE
> 	constant by mistake.

Yeah, I had a similar comment in the commit message (but much more verbose than your concise addition above), but I edited it several times, without finding a wording that I liked. I eventually removed it, because it didn't really add any value. :(

>> Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
> 
> With that addition to the commit message: ACK

This patch is now in the 'next' branch, so I guess it's too late to add this to the commit message (which I would be quite happy to do).

Well, at the beginning of the next cycle, 'next' will be rebuilt, so I guess (if we remember!) this patch could be updated then.

ATB, Ramsay Jones

Previous: Johannes SchindelinNext: Junio C Hamano
Message 3 of 6 in “archive-tar: fix a sparse 'constant too large' warning”
  1. archive-tar: fix a sparse 'constant too large' warningRamsay Jones, May 8, 2017
  2. Johannes SchindelinMay 9, 2017
  3. Ramsay JonesMay 9, 2017
  4. Junio C HamanoMay 9, 2017
  5. Johannes SchindelinMay 10, 2017
  6. Junio C HamanoMay 9, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.