{"thread":{"id":"46530","subject":"[PATCH] Fix delta integer overflows","startedAt":"2017-08-07T18:11:01Z","lastAt":"2017-08-08T11:39:04Z","messageCount":11,"participants":["Martin Koegler","Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"325711","messageId":"1502129437-31226-1-git-send-email-martin@mail.zuhause","threadId":"46530","inReplyTo":null,"subject":"[PATCH] Fix delta integer overflows","fromName":"Martin Koegler","fromEmail":"martin.koegler@chello.at","sentAt":"2017-08-07T18:10:37Z","receivedAt":"2017-08-07T18:11:01Z","isPatch":true,"sender":{"key":"martin.koegler@chello.at","avatar":"https://avatars.githubusercontent.com/u/1859030?v=4"},"body":"From: Martin Koegler <martin.koegler@chello.at>\n\nThe current delta code produces incorrect pack objects for files > 4GB.\n\nSigned-off-by: Martin Koegler <martin.koegler@chello.at>\n---\n diff-delta.c | 23 ++++++++++++-----------\n 1 file changed, 12 insertions(+), 11 deletions(-)\n\nJust pass any file > 4 GB to the delta-compression [by increasing the delta limits].\nAs file size, a truncated 32bit value will be encoded, leading to broken pack files.\n\ndiff --git a/diff-delta.c b/diff-delta.c\nindex 3797ce6..13e5a01 100644\n--- a/diff-delta.c\n+++ b/diff-delta.c\n@@ -319,7 +319,8 @@ create_delta(const struct delta_index *index,\n \t     const void *trg_buf, unsigned long trg_size,\n \t     unsigned long *delta_size, unsigned long max_size)\n {\n-\tunsigned int i, outpos, outsize, moff, msize, val;\n+\tunsigned int i, val;\n+\tunsigned long l, outpos, outsize, moff, msize;\n \tint inscnt;\n \tconst unsigned char *ref_data, *ref_top, *data, *top;\n \tunsigned char *out;\n@@ -336,20 +337,20 @@ create_delta(const struct delta_index *index,\n \t\treturn NULL;\n \n \t/* store reference buffer size */\n-\ti = index->src_size;\n-\twhile (i >= 0x80) {\n-\t\tout[outpos++] = i | 0x80;\n-\t\ti >>= 7;\n+\tl = index->src_size;\n+\twhile (l >= 0x80) {\n+\t\tout[outpos++] = l | 0x80;\n+\t\tl >>= 7;\n \t}\n-\tout[outpos++] = i;\n+\tout[outpos++] = l;\n \n \t/* store target buffer size */\n-\ti = trg_size;\n-\twhile (i >= 0x80) {\n-\t\tout[outpos++] = i | 0x80;\n-\t\ti >>= 7;\n+\tl = trg_size;\n+\twhile (l >= 0x80) {\n+\t\tout[outpos++] = l | 0x80;\n+\t\tl >>= 7;\n \t}\n-\tout[outpos++] = i;\n+\tout[outpos++] = l;\n \n \tref_data = index->src_buf;\n \tref_top = ref_data + index->src_size;\n-- \n2.1.4\n\n"},{"id":"325724","messageId":"xmqq1sonql76.fsf@gitster.mtv.corp.google.com","threadId":"46530","inReplyTo":"1502129437-31226-1-git-send-email-martin@mail.zuhause","subject":"Re: [PATCH] Fix delta integer overflows","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-07T18:36:29Z","receivedAt":"2017-08-07T18:36:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin Koegler <martin.koegler@chello.at> writes:\n\n> From: Martin Koegler <martin.koegler@chello.at>\n>\n> The current delta code produces incorrect pack objects for files > 4GB.\n>\n> Signed-off-by: Martin Koegler <martin.koegler@chello.at>\n> ---\n>  diff-delta.c | 23 ++++++++++++-----------\n>  1 file changed, 12 insertions(+), 11 deletions(-)\n>\n> Just pass any file > 4 GB to the delta-compression [by increasing the delta limits].\n> As file size, a truncated 32bit value will be encoded, leading to broken pack files.\n\nThe patch obviously makes the code better and self consistent in\nthat \"struct delta_index\" has src_size as ulong, and this function\ntakes trg_size as ulong, and it was plain wrong for the code to\nassume that \"i\", which is uint, can receive it safely.\n\nIn the longer term we might want to move to size_t or even\nuintmax_t, as the ulong on a platform may not be long enough in\norder to express the largest file size the platform can have, but\nthis patch (1) is good even without such a change, and (2) gives a\ngood foundation to build on if we want such a change on top.\n\nThanks.  Will queue.\n\n>\n> diff --git a/diff-delta.c b/diff-delta.c\n> index 3797ce6..13e5a01 100644\n> --- a/diff-delta.c\n> +++ b/diff-delta.c\n> @@ -319,7 +319,8 @@ create_delta(const struct delta_index *index,\n>  \t     const void *trg_buf, unsigned long trg_size,\n>  \t     unsigned long *delta_size, unsigned long max_size)\n>  {\n> -\tunsigned int i, outpos, outsize, moff, msize, val;\n> +\tunsigned int i, val;\n> +\tunsigned long l, outpos, outsize, moff, msize;\n>  \tint inscnt;\n>  \tconst unsigned char *ref_data, *ref_top, *data, *top;\n>  \tunsigned char *out;\n> @@ -336,20 +337,20 @@ create_delta(const struct delta_index *index,\n>  \t\treturn NULL;\n>  \n>  \t/* store reference buffer size */\n> -\ti = index->src_size;\n> -\twhile (i >= 0x80) {\n> -\t\tout[outpos++] = i | 0x80;\n> -\t\ti >>= 7;\n> +\tl = index->src_size;\n> +\twhile (l >= 0x80) {\n> +\t\tout[outpos++] = l | 0x80;\n> +\t\tl >>= 7;\n>  \t}\n> -\tout[outpos++] = i;\n> +\tout[outpos++] = l;\n>  \n>  \t/* store target buffer size */\n> -\ti = trg_size;\n> -\twhile (i >= 0x80) {\n> -\t\tout[outpos++] = i | 0x80;\n> -\t\ti >>= 7;\n> +\tl = trg_size;\n> +\twhile (l >= 0x80) {\n> +\t\tout[outpos++] = l | 0x80;\n> +\t\tl >>= 7;\n>  \t}\n> -\tout[outpos++] = i;\n> +\tout[outpos++] = l;\n>  \n>  \tref_data = index->src_buf;\n>  \tref_top = ref_data + index->src_size;\n"},{"id":"325730","messageId":"alpine.DEB.2.21.1.1708072136290.4271@virtualbox","threadId":"46530","inReplyTo":"xmqq1sonql76.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] Fix delta integer overflows","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-08-07T19:39:12Z","receivedAt":"2017-08-07T19:39:27Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 7 Aug 2017, Junio C Hamano wrote:\n\n> Martin Koegler <martin.koegler@chello.at> writes:\n> \n> > From: Martin Koegler <martin.koegler@chello.at>\n> >\n> > The current delta code produces incorrect pack objects for files > 4GB.\n> >\n> > Signed-off-by: Martin Koegler <martin.koegler@chello.at>\n> > ---\n> >  diff-delta.c | 23 ++++++++++++-----------\n> >  1 file changed, 12 insertions(+), 11 deletions(-)\n> >\n> > Just pass any file > 4 GB to the delta-compression [by increasing the delta limits].\n> > As file size, a truncated 32bit value will be encoded, leading to broken pack files.\n> \n> The patch obviously makes the code better and self consistent in\n> that \"struct delta_index\" has src_size as ulong, and this function\n> takes trg_size as ulong, and it was plain wrong for the code to\n> assume that \"i\", which is uint, can receive it safely.\n> \n> In the longer term we might want to move to size_t or even\n> uintmax_t, as the ulong on a platform may not be long enough in\n> order to express the largest file size the platform can have, but\n> this patch (1) is good even without such a change, and (2) gives a\n> good foundation to build on if we want such a change on top.\n> \n> Thanks.  Will queue.\n\nThis is sad. There is no \"may not be long enough\". We already know a\nplatform where unsigned long is not long enough, don't we? Why leave this\npatch in this intermediate state?\n\nIf you want to work on data in memory, then size_t is the appropriate data\ntype. We already use it elsewhere. Let's use it here, too, without the\nintermediate bump from the incorrect `int` to the equally incorrect\n`long`.\n\nCiao,\nJohannes\n"},{"id":"325733","messageId":"xmqqshh3p3b0.fsf@gitster.mtv.corp.google.com","threadId":"46530","inReplyTo":"alpine.DEB.2.21.1.1708072136290.4271@virtualbox","subject":"Re: [PATCH] Fix delta integer overflows","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-07T19:48:19Z","receivedAt":"2017-08-07T19:48:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n>> The patch obviously makes the code better and self consistent in\n>> that \"struct delta_index\" has src_size as ulong, and this function\n>> takes trg_size as ulong, and it was plain wrong for the code to\n>> assume that \"i\", which is uint, can receive it safely.\n>> \n>> In the longer term we might want to move to size_t or even\n>> uintmax_t, as the ulong on a platform may not be long enough in\n>> order to express the largest file size the platform can have, but\n>> this patch (1) is good even without such a change, and (2) gives a\n>> good foundation to build on if we want such a change on top.\n>> \n>> Thanks.  Will queue.\n>\n> This is sad. There is no \"may not be long enough\". We already know a\n> platform where unsigned long is not long enough, don't we? Why leave this\n> patch in this intermediate state?\n\nThis is a good foundation to build on, and I never said no further\nupdate on top of this patch is desired.  Look for \"(2)\" in what you\nquoted.\n"},{"id":"325744","messageId":"alpine.DEB.2.21.1.1708072307410.4271@virtualbox","threadId":"46530","inReplyTo":"xmqqshh3p3b0.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] Fix delta integer overflows","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-08-07T21:10:39Z","receivedAt":"2017-08-07T21:10:53Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Mon, 7 Aug 2017, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n> >> The patch obviously makes the code better and self consistent in\n> >> that \"struct delta_index\" has src_size as ulong, and this function\n> >> takes trg_size as ulong, and it was plain wrong for the code to\n> >> assume that \"i\", which is uint, can receive it safely.\n> >> \n> >> In the longer term we might want to move to size_t or even\n> >> uintmax_t, as the ulong on a platform may not be long enough in\n> >> order to express the largest file size the platform can have, but\n> >> this patch (1) is good even without such a change, and (2) gives a\n> >> good foundation to build on if we want such a change on top.\n> >> \n> >> Thanks.  Will queue.\n> >\n> > This is sad. There is no \"may not be long enough\". We already know a\n> > platform where unsigned long is not long enough, don't we? Why leave this\n> > patch in this intermediate state?\n> \n> This is a good foundation to build on, and I never said no further\n> update on top of this patch is desired.  Look for \"(2)\" in what you\n> quoted.\n\nSo are you saying that starting with v2.14.0, you accept patches into `pu`\nfor which you would previously have required multiple iterations before\neven considering it for `pu`?\n\nFrankly, I am a bit surprised that this obvious change from `unsigned\nlong` to `size_t` is not required in this case before queuing, but if the\nrules have changed to lower the bar for patch submissions, I am all for\nit. I always felt that we are wasting contributors' time a little too\nfreely and too deliberately.\n\nCiao,\nDscho\n"},{"id":"325762","messageId":"xmqqwp6fnjpg.fsf@gitster.mtv.corp.google.com","threadId":"46530","inReplyTo":"alpine.DEB.2.21.1.1708072307410.4271@virtualbox","subject":"Re: [PATCH] Fix delta integer overflows","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-07T21:36:59Z","receivedAt":"2017-08-07T21:37:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> So are you saying that starting with v2.14.0, you accept patches into `pu`\n> for which you would previously have required multiple iterations before\n> even considering it for `pu`?\n>\n> Frankly, I am a bit surprised that this obvious change from `unsigned\n> long` to `size_t` is not required in this case before queuing, but if the\n> rules have changed to lower the bar for patch submissions, I am all for\n> it. I always felt that we are wasting contributors' time a little too\n> freely and too deliberately.\n\nThis is not about where the bar is set.  It is about expectation.  I\ndo not expect much from occasional (or new) contributors and I try\nnot to demand too much from them.  The consequence is that as long\nas a small patch makes things better without making anything worse,\nI'd want to be inclusive to encourage them to build obvious\nimprovements on top.  Maybe they just want a single patch landed to\nfix their immediate needs (which may be generic enough and expected\nto be shared with many other people) without going further, so I may\nend up queuing something that only helps 40% of people until follow\nup patches are done to cover the remaining 60% of people, but that\nis fine as long as the patch does not make things worse (it is not\nlike a patch that helps 40% while hurting the remaining 60% until\na follow-up happens).\n\nI would expect a lot more from experienced contributors, when I know\nthey are capable of helping the remaining 60% inside the same series\nand without requiring too much hand-holding from me.  The same thing\nI cannot say to a occasional (or new) contributor---they may not be\ncoorperative, or they may be willing to do more but may require more\nhelp polishing their work than the bandwidth I can afford.\n\nSo if you are volunteering to help by either guiding Martin to aim\nhigher and make this a series with a larger and more complete scope,\nI'd very much appreciate it.  Or you can do a follow-up series on\ntop of what Martin posted.  Either is fine by me.  Just do not step\non each others' toes ;-)\n\nI avoided saying all of the above because I didn't want my word\ntaken out of context later to make it sound as if I were belittling\nthe competence of Martin, but you seem to want to force me to say\nthis, so here it is.\n\n"},{"id":"325774","messageId":"xmqqzibbm16o.fsf@gitster.mtv.corp.google.com","threadId":"46530","inReplyTo":"xmqqwp6fnjpg.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] Fix delta integer overflows","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-07T23:02:23Z","receivedAt":"2017-08-07T23:02:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> This is not about where the bar is set.  It is about expectation....\n\nAfter having thought about this a bit more, I think in the message I\nam responding to I mischaracterised the aspect of a patch that\ninfluences the \"expectation\".  It is much less about who the\ncontributor is but more about what the patch does.  \n\nIf the patch in question were from a more experienced contributor\n(like you or Peff), my internal reaction would have been \"gee, the\nsubmitter should have known better that a more complete fix should\ninvolve a larger integral type, not stopping at matching the largest\ntype that happens to be used in the interface without updating the\ninterface\".\n\nBut I still would have said that the patch is an improvement--as it\nindeed is; it does not make things worse anywhere and brings in a\nmore consistency.  And I still would have mentioned the same \"in the\nlonger term, we would want to use size_t or uintmax_t here, not just\nulong\".\n\nThe only thing I would have done differently if the submission were\nby a more experienced contributor is that I probably would have\nadded \"yes this may be an improvement, but I expected you should\nknow better to at least mention the longer term direction to use\nsize_t or uintmax_t in the log message, even if you didn't\nimmediately extend this patch into a more complete series\".\n\nThat one is a difference of expectation between an occasional\ncontributor and an experienced one.\n"},{"id":"325778","messageId":"xmqqvalyn89b.fsf@gitster.mtv.corp.google.com","threadId":"46530","inReplyTo":"xmqq1sonql76.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] Fix delta integer overflows","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-08T01:44:16Z","receivedAt":"2017-08-08T01:44:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Martin Koegler <martin.koegler@chello.at> writes:\n>\n>> From: Martin Koegler <martin.koegler@chello.at>\n>>\n>> The current delta code produces incorrect pack objects for files > 4GB.\n>>\n>> Signed-off-by: Martin Koegler <martin.koegler@chello.at>\n>> ---\n>>  diff-delta.c | 23 ++++++++++++-----------\n>>  1 file changed, 12 insertions(+), 11 deletions(-)\n>>\n>> Just pass any file > 4 GB to the delta-compression [by increasing the delta limits].\n>> As file size, a truncated 32bit value will be encoded, leading to broken pack files.\n>\n> The patch obviously makes the code better and self consistent in\n> that \"struct delta_index\" has src_size as ulong, and this function\n> takes trg_size as ulong, and it was plain wrong for the code to\n> assume that \"i\", which is uint, can receive it safely.\n>\n> In the longer term we might want to move to size_t or even\n> uintmax_t, as the ulong on a platform may not be long enough in\n> order to express the largest file size the platform can have, but\n> this patch (1) is good even without such a change, and (2) gives a\n> good foundation to build on if we want such a change on top.\n>\n> Thanks.  Will queue.\n\nHaving said that, I am a bit curious how you came to this patch.\nWas the issue found by code inspection, or did you actually have a\nreal life use case to raise the core.bigFileThreshold configuration\nto a value above 4GB?\n\nThanks.\n"},{"id":"325781","messageId":"20170808062047.GA4091@mail.zuhause","threadId":"46530","inReplyTo":"alpine.DEB.2.21.1.1708072136290.4271@virtualbox","subject":"Re: [PATCH] Fix delta integer overflows","fromName":"Martin Koegler","fromEmail":"martin.koegler@chello.at","sentAt":"2017-08-08T06:20:47Z","receivedAt":"2017-08-08T06:21:05Z","isPatch":true,"sender":{"key":"martin.koegler@chello.at","avatar":"https://avatars.githubusercontent.com/u/1859030?v=4"},"body":"On Mon, Aug 07, 2017 at 09:39:12PM +0200, Johannes Schindelin wrote:\n> If you want to work on data in memory, then size_t is the appropriate data\n> type. We already use it elsewhere. Let's use it here, too, without the\n> intermediate bump from the incorrect `int` to the equally incorrect\n> `long`.\n\nI disagree with \"We already use it elsewhere.\". The whole delta code uses \"unsigned long\" -\nlook at delta.h. Look at unpack-objects.c. Or cache.h. Or pack-objects.c. Or index-pack.c.\n\nOther possible cases:\ngit grep \"unsigned long\" |grep size\n\nSo the codebase still suggests, that \"unsigned long\" is the data type for storing object sizes.\n\nI would be fine with resubmitting a patch using size_t/off_t for the touched parts - changing the whole\ncore code is a too invasive change for a bug fix.\n\nRegards,\nMartin\n\n\nFrom d97a7810ff679dd939972c151bcf23c122cdc570 Mon Sep 17 00:00:00 2001\nFrom: Martin Koegler <martin.koegler@chello.at>\nDate: Mon, 7 Aug 2017 20:00:30 +0200\nSubject: [PATCH] Fix delta integer overflows\n\nThe current delta code produces incorrect pack objects for files > 4GB.\n\nSigned-off-by: Martin Koegler <martin.koegler@chello.at>\n---\n diff-delta.c | 24 +++++++++++++-----------\n 1 file changed, 13 insertions(+), 11 deletions(-)\n\ndiff --git a/diff-delta.c b/diff-delta.c\nindex 3797ce6..cd238c8 100644\n--- a/diff-delta.c\n+++ b/diff-delta.c\n@@ -319,7 +319,9 @@ create_delta(const struct delta_index *index,\n \t     const void *trg_buf, unsigned long trg_size,\n \t     unsigned long *delta_size, unsigned long max_size)\n {\n-\tunsigned int i, outpos, outsize, moff, msize, val;\n+\tunsigned int i, val;\n+\toff_t outpos, moff;\n+\tsize_t l, outsize, msize;\n \tint inscnt;\n \tconst unsigned char *ref_data, *ref_top, *data, *top;\n \tunsigned char *out;\n@@ -336,20 +338,20 @@ create_delta(const struct delta_index *index,\n \t\treturn NULL;\n \n \t/* store reference buffer size */\n-\ti = index->src_size;\n-\twhile (i >= 0x80) {\n-\t\tout[outpos++] = i | 0x80;\n-\t\ti >>= 7;\n+\tl = index->src_size;\n+\twhile (l >= 0x80) {\n+\t\tout[outpos++] = l | 0x80;\n+\t\tl >>= 7;\n \t}\n-\tout[outpos++] = i;\n+\tout[outpos++] = l;\n \n \t/* store target buffer size */\n-\ti = trg_size;\n-\twhile (i >= 0x80) {\n-\t\tout[outpos++] = i | 0x80;\n-\t\ti >>= 7;\n+\tl = trg_size;\n+\twhile (l >= 0x80) {\n+\t\tout[outpos++] = l | 0x80;\n+\t\tl >>= 7;\n \t}\n-\tout[outpos++] = i;\n+\tout[outpos++] = l;\n \n \tref_data = index->src_buf;\n \tref_top = ref_data + index->src_size;\n-- \n2.1.4\n\n"},{"id":"325782","messageId":"20170808062522.GB4091@mail.zuhause","threadId":"46530","inReplyTo":"xmqqvalyn89b.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] Fix delta integer overflows","fromName":"Martin Koegler","fromEmail":"martin.koegler@chello.at","sentAt":"2017-08-08T06:25:23Z","receivedAt":"2017-08-08T06:25:35Z","isPatch":true,"sender":{"key":"martin.koegler@chello.at","avatar":"https://avatars.githubusercontent.com/u/1859030?v=4"},"body":"On Mon, Aug 07, 2017 at 06:44:16PM -0700, Junio C Hamano wrote:\n> Having said that, I am a bit curious how you came to this patch.\n> Was the issue found by code inspection, or did you actually have a\n> real life use case to raise the core.bigFileThreshold configuration\n> to a value above 4GB?\n\nReal life use - tracking changes in larger files.\n\nRaising the limit above 4GB suddenly resulted in a broken pack files in the repository and\naborts of various git commands.\n\nData is still recoverable with all size sanity checks disabled.\n\nRegards,\nMartin\n"},{"id":"325802","messageId":"alpine.DEB.2.21.1.1708081336410.4271@virtualbox","threadId":"46530","inReplyTo":"20170808062047.GA4091@mail.zuhause","subject":"Re: [PATCH] Fix delta integer overflows","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-08-08T11:38:26Z","receivedAt":"2017-08-08T11:39:04Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Martin,\n\nOn Tue, 8 Aug 2017, Martin Koegler wrote:\n\n> On Mon, Aug 07, 2017 at 09:39:12PM +0200, Johannes Schindelin wrote:\n> > If you want to work on data in memory, then size_t is the appropriate data\n> > type. We already use it elsewhere. Let's use it here, too, without the\n> > intermediate bump from the incorrect `int` to the equally incorrect\n> > `long`.\n> \n> I disagree with \"We already use it elsewhere.\".\n\nBy \"it\" I meant \"size_t\".\n\n> The whole delta code uses \"unsigned long\" - look at delta.h. Look at\n> unpack-objects.c. Or cache.h. Or pack-objects.c. Or index-pack.c.\n\nI know that. It is a major bug in the source code.\n\n> Other possible cases:\n> git grep \"unsigned long\" |grep size\n\nYes, even more bugs.\n\n> So the codebase still suggests, that \"unsigned long\" is the data type\n> for storing object sizes.\n\nAnd it would be wrong, and we know it already. Most importantly, Junio\nknows it already.\n\n> I would be fine with resubmitting a patch using size_t/off_t for the\n> touched parts - changing the whole core code is a too invasive change\n> for a bug fix.\n\nSorry, my mistake: I never meant to burden you with the invasive change.\nI only wanted you to change the `int` to `size_t` right away.\n\nThanks,\nJohannes\n"}]}