shejialuo <shejialuo@xxxxxxxxx> writes:
> During fsck, we use "strbuf_read" to read the content of "packed-refs"
> without using mmap mechanism. This is a bad practice which would consume
> more memory than using mmap mechanism. Besides, as all code paths in
> "packed-backend.c" use this way, we should make "fsck" align with the
> current codebase.
>
> As we have introduced the helper function "allocate_snapshot_buffer", we
> could simple use this function to use mmap mechanism.
"could simple" -> "can simply".
> Suggested-by: Jeff King <peff@xxxxxxxx>
> Suggested-by: Patrick Steinhardt <ps@xxxxxx>
> Signed-off-by: shejialuo <shejialuo@xxxxxxxxx>
> ---
> refs/packed-backend.c | 19 +++++++------------
> 1 file changed, 7 insertions(+), 12 deletions(-)
Nice loss of line count ;-)
> - if (!st.st_size) {
> + if (!allocate_snapshot_buffer(&snapshot, fd, &st)) {
It is a bit funny to see that a helper function that works at a much
higher conceptual level treat an empty file so specially (namely,
should it be different from a header-only packed-refs file?). If I
were doing this refactoring in 2 & 3, I would probalby have made the
helper return "void", and have callers who do care about st.st_size
check that themselves.
But I'll let it pass.
> @@ -2121,21 +2121,16 @@ static int packed_fsck(struct ref_store *ref_store,
> goto cleanup;
> }
>
> - if (strbuf_read(&packed_ref_content, fd, 0) < 0) {
> - ret = error_errno(_("unable to read '%s'"), refs->path);
> - goto cleanup;
> - }
> -
> - ret = packed_fsck_ref_content(o, ref_store, &sorted, packed_ref_content.buf,
> - packed_ref_content.buf + packed_ref_content.len);
> + ret = packed_fsck_ref_content(o, ref_store, &sorted, snapshot.start,
> + snapshot.eof);
> if (!ret && sorted)
> - ret = packed_fsck_ref_sorted(o, ref_store, packed_ref_content.buf,
> - packed_ref_content.buf + packed_ref_content.len);
> + ret = packed_fsck_ref_sorted(o, ref_store, snapshot.start,
> + snapshot.eof);
>
> cleanup:
> if (fd >= 0)
> close(fd);
> - strbuf_release(&packed_ref_content);
> + clear_snapshot_buffer(&snapshot);
> return ret;
> }
