Re: [PATCH 06/10] packed-backend: add "packed-refs" entry consistency check
- From
shejialuo <shejialuo@gmail.com>
- Date
- Jan 17, 2025, 14:35 UTC
- Message-ID
- <Z4pqv2JMNgIgFgG0@ArchLinux>
- In-Reply-To
- <Z4kQV4Nve632rJ3s@pks.im>
On Thu, Jan 16, 2025 at 02:57:43PM +0100, Patrick Steinhardt wrote:
Show 56 quoted lines
> On Sun, Jan 05, 2025 at 09:49:59PM +0800, shejialuo wrote:
> > diff --git a/refs/packed-backend.c b/refs/packed-backend.c
> > index f6142a4402..6e521a9f87 100644
> > --- a/refs/packed-backend.c
> > +++ b/refs/packed-backend.c
> > @@ -1822,7 +1822,96 @@ static int packed_fsck_ref_header(struct fsck_options *o, const char *start, con
> > return 0;
> > }
> >
> > +static int packed_fsck_ref_peeled_line(struct fsck_options *o,
> > + struct ref_store *ref_store, int line_number,
> > + const char *start, const char *eol)
> > +{
> > + struct strbuf peeled_entry = STRBUF_INIT;
> > + struct fsck_ref_report report = { 0 };
> > + struct object_id peeled;
> > + const char *p;
> > + int ret = 0;
> > +
> > + strbuf_addf(&peeled_entry, "packed-refs line %d", line_number);
> > + report.path = peeled_entry.buf;
> > +
> > + start++;
> > + if (parse_oid_hex_algop(start, &peeled, &p, ref_store->repo->hash_algo)) {
> > + ret |= fsck_report_ref(o, &report,
> > + FSCK_MSG_BAD_PACKED_REF_ENTRY,
> > + "'%.*s' has invalid peeled oid",
> > + (int)(eol - start), start);
> > + goto cleanup;
> > + }
> > +
> > + if (p != eol) {
> > + ret |= fsck_report_ref(o, &report,
> > + FSCK_MSG_BAD_PACKED_REF_ENTRY,
> > + "has trailing garbage after peeled oid '%.*s'",
> > + (int)(eol - p), p);
> > + goto cleanup;
> > + }
> > +
> > +cleanup:
> > + strbuf_release(&peeled_entry);
> > + return ret;
> > +}
> > +
> > +static int packed_fsck_ref_main_line(struct fsck_options *o,
> > + struct ref_store *ref_store, int line_number,
> > + const char *start, const char *eol)
> > +{
> > + struct strbuf packed_entry = STRBUF_INIT;
> > + struct fsck_ref_report report = { 0 };
> > + struct strbuf refname = STRBUF_INIT;
>
> It feels quite inefficient to create a separate buffer for every
> invocation of this function, as there can be many million refs in a
> repo. Might be something to avoid by passing in a scratch buffer.
> I see. I will improve this in the next version.
Thanks, Jialuo