On Tue, Feb 15, 2022 at 9:48 PM John Cai <johncai86@xxxxxxxxx> 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@xxxxxxxxxxxxxx/ 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.
