Re: [PATCH v7 4/4] cat-file: add --batch-command mode

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Feb 16, 2022, 03:17 UTC
Message-ID: <CAPig+cSEO407rp3_ufS_DV1XJnkkb++6MP8UJhpj5aQP04gZ-Q@mail.gmail.com>
In-Reply-To: <xmqq4k4zwm65.fsf@gitster.g>

On Tue, Feb 15, 2022 at 10:00 PM Junio C Hamano <gitster@pobox.com> wrote:

Show 17 quoted lines

> John Cai <johncai86@gmail.com> writes:
> > 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.
>
> Oh, I see.
>
> I do not see any issue with separation of concerns here, actually.
>
> As long as "dispatch_calls() consumes all the cmd[] before it
> returns to the caller" is clearly understood between the function
> and its caller(s) [*], clearing of "nr" the caller has is entirely
> caller's problem.  It becomes needed only because this caller
> decides to reuse cmd[] array.
>
>         Side note: you do have a comment before the function to tell
>         what to expect out of the helper for its callers, right?

True, it is the caller's problem due to reusing the array, though dispatch_calls() freeing `line` muddies things slightly. A comment before the function could indeed be helpful, especially letting callers know that `line` is being freed.

It probably also would be wise to replace the free() call with FREE_AND_NULL() in order to address the dangling-pointer concern.