From: Johannes Sixt <j6t@kdbg.org>
Date: Sat, 13 Oct 2018 08:43:24 GMT
Subject: Re: [PATCH] zlib.c: use size_t for size
Message-ID: <2e7b2f4b-92c3-a583-5985-38090cc78c64@kdbg.org>
In-Reply-To: <20181013024624.GB15595@sigill.intra.peff.net>

Am 13.10.18 um 04:46 schrieb Jeff King:
> But no, right before that we have this line:
> 
>            offset -= win->offset;
> 
> So offset is in fact no longer its original meaning of "offset into the
> packfile", but is now an offset of the specific request into the window
> we found.
> 
> So I think it's correct, but it sure confused me. I wonder if another
> variable might help, like:
> 
>    size_t offset_in_window;
>    ...
> 
>    /*
>     * We know this difference will fit in a size_t, because our mmap
>     * window by * definition can be no larger than a size_t.
>     */
>    offset_in_window = xsize_t(offset - win->offset);
>    if (left)
> 	*left = win->len - offset_in_window;
>    return win->base + offset_in_window;
> 
> I dunno. Maybe it is overkill.

Thank you for your analysis. No, I don't think that such a new variable 
would be overkill. It is important to make such knowledge of value 
magnitudes explicit precisely because it reduces confusion and helps 
reviewers of the code verify correctness.

-- Hannes