On Mon, Nov 16, 2020 at 11:39:35AM -0800, Junio C Hamano wrote: > Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes: > > > On Fri, Nov 13 2020, Patrick Steinhardt wrote: > > > >> While not document, it is currently possible to specify config entries > > > > "While not documented..." > > > >> + strbuf_addf(&envvar, "GIT_CONFIG_KEY_%d", i); > >> + if ((key = getenv(envvar.buf)) == NULL) > >> + break; > > > > The convention in git.git is to avoid explicit NULL checks. So maybe > > something like this, which also avoids the assignment inside an "if" > > > > key = getenv(envvar.buf); > > if (!key) > > break; > > All good suggestions, but... > > "While not documented" yes, for sure, but we do not document it for > a good reason---it is a pure implementation detail between Git > process that runs another one as its internal implementation detail. Agreed > I especially do not think we want to read from unbounded number of > GIT_CONFIG_KEY_<N> variables like this patch does. How would a > script cleanse its environment to protect itself from stray such > environment variable pair? Count up from 1 to forever? Run "env" > and grep for "GIT_CONFIG_KEY_[0-9]*=" (the answer is No. What if > some environment variables have newline in its values?) You only have to unset `GIT_CONFIG_KEY_1` as the parser will stop iterating on the first missing key. More generally, if you set `n` keys, it's sufficient to unset key `n+1`. Patrick
Attachment:
signature.asc
Description: PGP signature
