Re: Re* [PATCH v5] describe: refresh the index when 'broken' flag is used

[Junio C Hamano <gitster@pobox.com>]

Junio C Hamano <gitster@pobox.com> writes:

> Abhijeet Sonar <abhijeet.nkt@gmail.com> writes:
>
>> To me, this looks much better.  child_process_clear's name already
>> suggests that is sort of like a destructor, so it makes sense to
>> re-initialize everything here.  I even wonder why it was not that way to
>> begin with.  I suppose no callers are assuming that it only clears args
>> and env though?
>
> I guess that validating that supposition is a prerequisite to
> declare the change as "much better" and "makes sense".

Having said that, a lot more plausible reason is that most callers
are expected to use a single struct just once without reusing it, so
the cost of re-initialization is wasted cycles for them if it were
part of child_process_clear() which is primarily about releasing
resources (instead of leaking them).

It can be argued that it is a sensible design decision to make those
minority callers that do reuse the same struct to pay the cost of
reinitialization themselves, instead of the majority callers that
use it once and then release resources held in there.