On Fri, Oct 29 2021, Junio C Hamano wrote:
> Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes:
>
>>> + if (!git_env_bool("GIT_TRACE_REDACT", 1) || !nurl) {
>>> + die("Unable to get pack file %s\n%s", preq->url,
>>> + curl_errorstr);
>>
>> small nit: arrange if's from "if (cheap || expensive)", i.e. no need for
>> getenv() if !nurl, but maybe compilers are smart enough for that...
>
> They typically do not see what happens inside git_env_bool() while
> compling this compilation unit, and cannot tell if the programmer
> wanted to call it first for its side effects, hence they cannot
> swap them safely.
*nod*, but since that function is just:
int git_env_bool(const char *k, int def)
{
const char *v = getenv(k);
return v ? git_config_bool(k, v) : def;
}
I was hedging and pondering if some compilers were smart enough these
days to optimize things like that.
I.e. in this case getenv() is a simple C library function, the env
variable is constant, and we do a boolean test of it before calling
git_config_bool().
So a sufficiently smart compiler could turn that into:
/* global, probably something iterated over env already */
static int __have_seen_GIT_TRACE_REDACT = 0;
...
if ((!__have_seen_GIT_TRACE_REDACT || !nurl) ||
(__have_seen_GIT_TRACE_REDACT && git_env_bool_without_v_bool_check(...)))
But probably not, since it wolud need quite a bit of C library
cooperation/hooks...
