Re: [PATCH v5 05/16] bisect--helper: make `terms` an explicit singleton

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

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> I have no strong preference, but I do not think that it particularly
>> contributes to "clarifying that we are using global state" to make
>> direct accesses to the variable everywhere.  I dunno.
>
> I do have a strong preference to avoid mixing and matching global
> variables with functions that pretend not to use said global variables, as
> it is too easy to forget to pass through the corresponding parameter to a
> function at a lower layer when that function accesses the global variable
> anyway.

When you have a truly well "libified" API and a program that needs
to use global to communicate to other parts of the system, then the
approach I suggested would follow a much better design taste.  For
example, if your program takes an argument --option=string, and you
for whatever reason have to use a global variable to store the value
due to limitation of your option parsing API, you'd create a global
variable to store the string.  But well libified API functions (say,
the ones from the C library) do not have to care if you are using a
global or an on-stack variable.  I would not rewrite strlen(const
char *) to my_strlen(void) that only looks at the global variable
and use it instead.

But it is quite possible that bisect--helper may not have such a
clean interface, in which case it is pefectly OK for everybody to
look at the global directly.  My comment was mostly a reaction based
on the assumption that your earlier "libifyable state" comment meant
that these call graphs are nicely libified and pass everything they
need through via parameters, in which case, the ultimate callers of
these helper functions that take "struct bisect_terms *", like
free_terms(), may have the parameter they pass down to the callchain
on their stack, and now have to pass a pointer to a global, without
the callchain that is "as libified as possible" having to know.