From: Eric Sunshine Date: Wed, 16 Feb 2022 03:01:07 GMT Subject: Re: [PATCH v7 4/4] cat-file: add --batch-command mode Message-ID: In-Reply-To: On Tue, Feb 15, 2022 at 9:48 PM John Cai wrote: > On 15 Feb 2022, at 20:28, Junio C Hamano wrote: > > and make the caller do > > > > dispatch_calls(opt, ..., nr); > > nr = 0; > > > > instead of passing a pointer to nr like the posted patch? > > Yeah, this is what I had before but there was discussion about separation of concerns in [1]. But perhaps it's preferable compared to passing a pointer to nr. > > 1. https://lore.kernel.org/git/CAPig+cTwLhn1GZ_=6s0FXL0z=Q=p1w9ZGK0hAV8wfK9RsQYjnA@mail.gmail.com/ My biggest concern when mentioning it during review was that if a caller forgets to do `nr = 0`, then a sequence such as: dispatch_calls(...); ... dispatch_calls(...); will send dangling pointers to the command handlers in the second dispatch_calls() invocation because the first call to dispatch_calls() did free(cmd[i].line). In that sense, it's an accident waiting to happen if people modifying this code in the future aren't paying close attention. That said, I don't feel too strongly about it and mentioned in my review that it might be "good enough" as-is (with the caller having to remember to `nr = 0`) since it's a local helper function.