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

Reply via email to