Re: [RFC/PATCH] pager.c: replace git_config with git_config_get_string

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Karsten Blees <karsten.blees@xxxxxxxxx> writes:

> Am 27.06.2014 13:55, schrieb Matthieu Moy:
>> Karsten Blees <karsten.blees@xxxxxxxxx> 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.

>> 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/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]