Re: [RFC/PATCH] pager.c: replace git_config with git_config_get_string
- From
Matthieu Moy <matthieu.moy@grenoble-inp.fr>
- Date
- Jun 27, 2014, 19:19 UTC
- Message-ID
- <vpqmwcyw47q.fsf@anie.imag.fr>
- In-Reply-To
- <53ADA26E.8030801@gmail.com>
Karsten Blees <karsten.blees@gmail.com> writes:
Show 18 quoted lines
> Am 27.06.2014 13:55, schrieb Matthieu Moy: >> Karsten Blees <karsten.blees@gmail.com> writes: >> >>> If for some reason a config string is accessed after config_cache_free() >>> (which would be a bug), you won't notice if strings are xstrdup()ed (i.e. git >>> will continue to run with some invalid configuration). This is IMO much worse >>> than failing with segfault. >> >> I disagree. In most case, continuing to use the old config value is the >> right thing. >> >> First, config_cache_free() is called whenever _any_ configuration >> variable is set. > > This is illogical. An API should do the right thing, and if the > right thing is to keep the old values, as you pointed out, then > setting a config value shouldn't invalidate the cache (at least > not automatically).
The reason for which setting any config value invalidates the cache is that it is _much_ simpler to implement. Less complexity, less corner-cases, less bugs.
Show 6 quoted lines
>> So, setting "core.foo" also invalidates "core.bar" in >> the cache, while a user of "core.bar" could continue working with the >> old, unchanged value. > > But a user of an xstrdup()ed copy of "core.foo" would also continue > to work with the old value.
Exactly like the current code does. Has it ever been even close to being a problem?
What you're suggesting brings a lot more complexity in the code, and I can't imagine a case where it would have a real benefit. So, why bother?
-- Matthieu Moy http://www-verimag.imag.fr/~moy/