Hi Junio,
On Fri, 12 Oct 2018, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes:
>
> > Hi Junio,
> >
> > On Fri, 12 Oct 2018, Junio C Hamano wrote:
> >
> >> From: Martin Koegler <martin.koegler@xxxxxxxxx>
> >> Date: Thu, 10 Aug 2017 20:13:08 +0200
> >>
> >> Signed-off-by: Martin Koegler <martin.koegler@xxxxxxxxx>
> >> Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
> >> ---
> >>
> >> * I made minimal adjustments to make the change apply to today's
> >> codebase. I still find some choices and mixing of off_t and
> >> size_t done by the patch a bit iffy, and some arith may need to
> >> become st_addX(). Extra set of eyes are very much appreciated.
> >>
> >> builtin/pack-objects.c | 10 +++++-----
> >> cache.h | 10 +++++-----
> >> pack-check.c | 6 +++---
> >> pack.h | 2 +-
> >> packfile.h | 2 +-
> >> wrapper.c | 8 ++++----
> >> zlib.c | 8 ++++----
> >> 7 files changed, 23 insertions(+), 23 deletions(-)
> >>
> >> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> >> index e6316d294d..b9ca04eb8a 100644
> >> --- a/builtin/pack-objects.c
> >> +++ b/builtin/pack-objects.c
> >> @@ -266,15 +266,15 @@ static void copy_pack_data(struct hashfile *f,
> >> struct packed_git *p,
> >> struct pack_window **w_curs,
> >> off_t offset,
> >> - off_t len)
> >> + size_t len)
> >> {
> >> unsigned char *in;
> >> - unsigned long avail;
> >> + size_t avail;
> >>
> >> while (len) {
> >> in = use_pack(p, w_curs, offset, &avail);
> >> if (avail > len)
> >
> > Do we have to be careful to handle sizeof(off_t) != sizeof(size_t) here? I
> > guess we would receive a compiler warning about different sizes in that
> > case, but it makes me worry.
>
> Yup, you just said exactly the same thing as I already said in the
> part you quoted. I still find the mixed use of off_t and size_t in
> this patch iffy, and that was the secondary reason why the patch was
> kept in the stalled state for so long. The primary reason was that
> nobody tried to dust it off and reignite the topic so far---which I
> am trying to correct, but as I said, this is just minimally adjusted
> to today's codebase, without any attempt to improve relative to the
> original patch.
>
> I think we are in agreement in that this is not making things worse,
> as we are already in the mixed arith territory before this patch.
I will *try* to find the time to audit this some more, then. Maybe next
week, maybe the one afterwards... ;-)
Ciao,
Dscho
>
>
