On Wed, May 07, 2025 at 10:53:56PM +0800, shejialuo wrote:
> @@ -761,19 +782,8 @@ static struct snapshot *create_snapshot(struct packed_ref_store *refs)
> verify_buffer_safe(snapshot);
> }
>
> - if (mmap_strategy != MMAP_OK && snapshot->mmapped) {
> - /*
> - * We don't want to leave the file mmapped, so we are
> - * forced to make a copy now:
> - */
> - size_t size = snapshot->eof - snapshot->start;
> - char *buf_copy = xmalloc(size);
> -
> - memcpy(buf_copy, snapshot->start, size);
> - clear_snapshot_buffer(snapshot);
> - snapshot->buf = snapshot->start = buf_copy;
> - snapshot->eof = buf_copy + size;
> - }
> + if (mmap_strategy == MMAP_TEMPORARY && snapshot->mmapped)
> + munmap_temporary_snapshot(snapshot);
The original triggers this conditional whenever the strategy is not
MMAP_OK (so MMAP_TEMPORARY or MMAP_NONE). But in your post-image, we do
so only for MMAP_TEMPORARY.
I can guess that the two end up the same, because snapshot->mmapped
would never be set when MMAP_NONE is set. But if we are going to make
such a logical inference, it should be explained in the commit message
(though my preference is to leave the code as-is, or to pull the
refactor into its own commit).
-Peff
