On Tue, Oct 17, 2023 at 07:18:04PM -0700, Junio C Hamano wrote: > Taylor Blau <me@xxxxxxxxxxxx> writes: > > > bulk-checkin.c | 118 +++++++++++++++++++++++++++++++++++++++++++++++++ > > bulk-checkin.h | 4 ++ > > 2 files changed, 122 insertions(+) > > Unlike the previous four, which were very clear refactoring to > create reusable helper functions, this step leaves a bad aftertaste > after reading twice, and I think what is disturbing is that many new > lines are pretty much literally copied from stream_blob_to_pack(). > > I wonder if we can introduce an "input" source abstraction, that > replaces "fd" and "size" (and "path" for error reporting) parameters > to the stream_blob_to_pack(), so that the bulk of the implementation > of stream_blob_to_pack() can call its .read() method to read bytes > up to "size" from such an abstracted interface? That would be a > good sized first half of this change. Then in the second half, you > can add another "input" source that works with in-core "buf" and > "size", whose .read() method will merely be a memcpy(). Thanks, I like this idea. I had initially avoided it in the first couple of rounds, because the abstraction felt clunky and involved an unnecessary extra memcpy(). But having applied your suggestion here, I think that the price is well worth the result, which is that `stream_blob_to_pack()` does not have to be implemented twice with very subtle differences. Thanks again for the suggestion, I'm really pleased with how it came out. Reroll coming shortly... Thanks, Taylor
