From: Karthik Nayak Date: Wed, 07 Oct 2026 09:25:28 GMT Subject: Re: [PATCH v3] packed-refs: use `fwrite()` when passing refs verbatim Message-ID: In-Reply-To: Patrick Steinhardt writes: > On Tue, Oct 06, 2026 at 05:11:57PM -0400, Karthik Nayak wrote: >> Patrick Steinhardt writes: >> > On Tue, Oct 06, 2026 at 11:18:40AM +0200, Karthik Nayak wrote: >> >> diff --git a/refs/packed-backend.c b/refs/packed-backend.c >> >> index a73fc6aca7..43ad674cf4 100644 >> >> --- a/refs/packed-backend.c >> >> +++ b/refs/packed-backend.c >> >> @@ -879,6 +879,12 @@ struct packed_ref_iterator { >> >> /* The current position in the snapshot's buffer: */ >> >> const char *pos; >> >> >> >> + /* >> >> + * Start of the current record, set when advancing `pos`. Used to >> >> + * pass records verbatim to `fwrite()`. >> >> + */ >> >> + const char *record_start; >> > >> > The way this is written makes you think that `pos == record_start`, and >> > thus one wonders why we even need this separate variable in the first >> > place. So I assume that we modify `pos` in some cases without modifying >> > the new variable at the same point in time. But if so, the above comment >> > is not true anymore. >> > >> >> Hmm. I only state that this is 'set _when_ advancing `pos`', Why do you >> think that this would mean `pos == record_start`? > > To me it reads as "whenever we advance `pos`, then we set > `record_start`". Which is not the case, we also sometimes advance `pos` > without setting it. > > Patrick Okay, let me simplify it and simply state its purpose, its implementation will anyways be traceable via the code. Thanks