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

Re: [PATCH] Fix delta integer overflows

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
Aug 7, 2017, 19:39 UTC
Message-ID
<alpine.DEB.2.21.1.1708072136290.4271@virtualbox>
In-Reply-To
<xmqq1sonql76.fsf@gitster.mtv.corp.google.com>
Hi,
On Mon, 7 Aug 2017, Junio C Hamano wrote:
Show 26 quoted lines
> Martin Koegler <martin.koegler@chello.at> writes:
> 
> > From: Martin Koegler <martin.koegler@chello.at>
> >
> > The current delta code produces incorrect pack objects for files > 4GB.
> >
> > Signed-off-by: Martin Koegler <martin.koegler@chello.at>
> > ---
> >  diff-delta.c | 23 ++++++++++++-----------
> >  1 file changed, 12 insertions(+), 11 deletions(-)
> >
> > Just pass any file > 4 GB to the delta-compression [by increasing the delta limits].
> > As file size, a truncated 32bit value will be encoded, leading to broken pack files.
> 
> The patch obviously makes the code better and self consistent in
> that "struct delta_index" has src_size as ulong, and this function
> takes trg_size as ulong, and it was plain wrong for the code to
> assume that "i", which is uint, can receive it safely.
> 
> In the longer term we might want to move to size_t or even
> uintmax_t, as the ulong on a platform may not be long enough in
> order to express the largest file size the platform can have, but
> this patch (1) is good even without such a change, and (2) gives a
> good foundation to build on if we want such a change on top.
> 
> Thanks.  Will queue.

This is sad. There is no "may not be long enough". We already know a platform where unsigned long is not long enough, don't we? Why leave this patch in this intermediate state?

If you want to work on data in memory, then size_t is the appropriate data type. We already use it elsewhere. Let's use it here, too, without the intermediate bump from the incorrect `int` to the equally incorrect `long`.

Ciao, Johannes

Previous: Junio C HamanoNext: Junio C Hamano
Message 3 of 11 in “Fix delta integer overflows”
  1. Fix delta integer overflowsMartin Koegler, Aug 7, 2017
  2. Junio C HamanoAug 7, 2017
  3. Johannes SchindelinAug 7, 2017
  4. Junio C HamanoAug 7, 2017
  5. Johannes SchindelinAug 7, 2017
  6. Junio C HamanoAug 7, 2017
  7. Junio C HamanoAug 7, 2017
  8. Martin KoeglerAug 8, 2017
  9. Johannes SchindelinAug 8, 2017
  10. Junio C HamanoAug 8, 2017
  11. Martin KoeglerAug 8, 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.