Re: [PATCH v3 2/3] config: add hashtable for config parsing retrieval

2014-06-27 Thread Junio C Hamano
Matthieu Moy matthieu@grenoble-inp.fr writes: Isn't that a bit overkill? Why not just let the caller manage a hashmap directly instead of a config_set? Because I had an experience under my belt of a painful refactoring of the_index which turned out to be not just a single array, I

Re: [PATCH v3 2/3] config: add hashtable for config parsing retrieval

2014-06-26 Thread Matthieu Moy
Ramsay Jones ram...@ramsay1.demon.co.uk writes: Hmm, maybe. The ... take advantage of the new code refers to the possibility (or otherwise) of re-using your work to update these older API functions to the new API style. (also, see Junio's response). I agree that, while caching the usual

Re: [PATCH v3 2/3] config: add hashtable for config parsing retrieval

2014-06-26 Thread Matthieu Moy
Tanay Abhra tanay...@gmail.com writes: For the config_cache_free(), would this change be enough? +static void config_cache_free(void) +{ + struct hashmap *config_cache; + struct config_cache_entry *entry; + struct hashmap_iter iter; + if (!hashmap_initialized) +

Re: [PATCH v3 2/3] config: add hashtable for config parsing retrieval

2014-06-26 Thread Matthieu Moy
Tanay Abhra tanay...@gmail.com writes: +Querying For Specific Variables +--- + +For programs wanting to query for specific variables in a non-callback +manner, the config API provides two functions `git_config_get_string` +and `git_config_get_string_multi`.They

Re: [PATCH v3 2/3] config: add hashtable for config parsing retrieval

2014-06-26 Thread Matthieu Moy
Junio C Hamano gits...@pobox.com writes: When the submodule script that uses git config -f .gitmodules is converted into C, if the updated config API is ready, it may be able to do something like these in a single program: const char *url; struct config_set *gm_config;

Re: [PATCH v3 2/3] config: add hashtable for config parsing retrieval

2014-06-26 Thread Junio C Hamano
Matthieu Moy matthieu@grenoble-inp.fr writes: Junio C Hamano gits...@pobox.com writes: When the submodule script that uses git config -f .gitmodules is converted into C, if the updated config API is ready, it may be able to do something like these in a single program: const char

Re: [PATCH v3 2/3] config: add hashtable for config parsing retrieval

2014-06-26 Thread Karsten Blees
Am 26.06.2014 21:00, schrieb Junio C Hamano: Matthieu Moy matthieu@grenoble-inp.fr writes: Junio C Hamano gits...@pobox.com writes: When the submodule script that uses git config -f .gitmodules is converted into C, if the updated config API is ready, it may be able to do something like

Re: [PATCH v3 2/3] config: add hashtable for config parsing retrieval

2014-06-26 Thread Junio C Hamano
Karsten Blees karsten.bl...@gmail.com writes: Because I had an experience under my belt of a painful refactoring of the_index which turned out to be not just a single array, I simply suspect that the final data structure to represent a set of config-like things will not be just a single

Re: [PATCH v3 2/3] config: add hashtable for config parsing retrieval

2014-06-25 Thread Junio C Hamano
Ramsay Jones ram...@ramsay1.demon.co.uk writes: On 24/06/14 00:25, Junio C Hamano wrote: ... Yup, that is a very good point. There needs an infrastructure to tie a set of files (i.e. the standard one being the chain of system-global /etc/gitconfig to repo-specific .git/config, and any

Re: [PATCH v3 2/3] config: add hashtable for config parsing retrieval

2014-06-25 Thread Junio C Hamano
Tanay Abhra tanay...@gmail.com writes: What changes should I implement in this series? Should I add new user facing API adding to the singleton callers which are already in this series. I think the underlying data structures that represent what the entire set of config data is needs to be

Re: [PATCH v3 2/3] config: add hashtable for config parsing retrieval

2014-06-25 Thread Karsten Blees
Am 25.06.2014 20:13, schrieb Junio C Hamano: Ramsay Jones ram...@ramsay1.demon.co.uk writes: On 24/06/14 00:25, Junio C Hamano wrote: ... Yup, that is a very good point. There needs an infrastructure to tie a set of files (i.e. the standard one being the chain of system-global

Re: [PATCH v3 2/3] config: add hashtable for config parsing retrieval

2014-06-25 Thread Karsten Blees
Am 24.06.2014 14:06, schrieb Tanay Abhra: On 6/23/2014 5:25 PM, Matthieu Moy wrote: Tanay Abhra tanay...@gmail.com writes: +/* for NULL values, 'util' for each `string_list_item` is flagged as 1 */ It's a void *, so just saying that it is flagged as 1 does not say how it's encoded. How

Re: [PATCH v3 2/3] config: add hashtable for config parsing retrieval

2014-06-25 Thread Junio C Hamano
Karsten Blees karsten.bl...@gmail.com writes: Am 25.06.2014 20:13, schrieb Junio C Hamano: Ramsay Jones ram...@ramsay1.demon.co.uk writes: I had expected to see one hash table per file/blob, with the three standard config hash tables linked together to implement the scope/ priority rules.

Re: [PATCH v3 2/3] config: add hashtable for config parsing retrieval

2014-06-25 Thread Karsten Blees
Am 23.06.2014 12:11, schrieb Tanay Abhra: [...] + +static struct config_cache_entry *config_cache_find_entry(const char *key) +{ + struct hashmap *config_cache; + struct config_cache_entry k; + struct config_cache_entry *found_entry; + char *normalized_key; + int ret;

Re: [PATCH v3 2/3] config: add hashtable for config parsing retrieval

2014-06-24 Thread Tanay Abhra
On 6/24/2014 4:55 AM, Junio C Hamano wrote: Ramsay Jones ram...@ramsay1.demon.co.uk writes: +static struct hashmap *get_config_cache(void) +{ + static struct hashmap config_cache; + if (!hashmap_initialized) { + config_cache_init(config_cache); +

Re: [PATCH v3 2/3] config: add hashtable for config parsing retrieval

2014-06-24 Thread Tanay Abhra
On 6/23/2014 5:25 PM, Matthieu Moy wrote: Tanay Abhra tanay...@gmail.com writes: +/* for NULL values, 'util' for each `string_list_item` is flagged as 1 */ It's a void *, so just saying that it is flagged as 1 does not say how it's encoded. How about ... is an 'int *' pointing to the value

Re: [PATCH v3 2/3] config: add hashtable for config parsing retrieval

2014-06-24 Thread Tanay Abhra
On 6/24/2014 4:44 AM, Junio C Hamano wrote: Tanay Abhra tanay...@gmail.com writes: +static int hashmap_initialized; + ... +static struct hashmap *get_config_cache(void) +{ + static struct hashmap config_cache; + if (!hashmap_initialized) { +

Re: [PATCH v3 2/3] config: add hashtable for config parsing retrieval

2014-06-24 Thread Ramsay Jones
On 23/06/14 17:20, Tanay Abhra wrote: On 06/23/2014 07:57 AM, Ramsay Jones wrote: On 23/06/14 11:11, Tanay Abhra wrote: [snip] +static struct hashmap *get_config_cache(void) +{ + static struct hashmap config_cache; + if (!hashmap_initialized) { +

Re: [PATCH v3 2/3] config: add hashtable for config parsing retrieval

2014-06-24 Thread Ramsay Jones
On 24/06/14 00:25, Junio C Hamano wrote: Ramsay Jones ram...@ramsay1.demon.co.uk writes: +static struct hashmap *get_config_cache(void) +{ + static struct hashmap config_cache; + if (!hashmap_initialized) { + config_cache_init(config_cache); + hashmap_initialized =

[PATCH v3 2/3] config: add hashtable for config parsing retrieval

2014-06-23 Thread Tanay Abhra
Currently `git_config()` uses a callback mechanism and file rereads for config values. Due to this approach, it is not uncommon for the config files to be parsed several times during the run of a git program, with different callbacks picking out different variables useful to themselves. Add a

Re: [PATCH v3 2/3] config: add hashtable for config parsing retrieval

2014-06-23 Thread Matthieu Moy
Tanay Abhra tanay...@gmail.com writes: +/* for NULL values, 'util' for each `string_list_item` is flagged as 1 */ It's a void *, so just saying that it is flagged as 1 does not say how it's encoded. How about ... is an 'int *' pointing to the value 1. And actually, you can save one malloc by

Re: [PATCH v3 2/3] config: add hashtable for config parsing retrieval

2014-06-23 Thread Ramsay Jones
On 23/06/14 11:11, Tanay Abhra wrote: [snip] diff --git a/config.c b/config.c index a1aef1c..6200f36 100644 --- a/config.c +++ b/config.c @@ -9,6 +9,8 @@ #include exec_cmd.h #include strbuf.h #include quote.h +#include hashmap.h +#include string-list.h struct config_source {

Re: [PATCH v3 2/3] config: add hashtable for config parsing retrieval

2014-06-23 Thread Tanay Abhra
On 06/23/2014 07:57 AM, Ramsay Jones wrote: On 23/06/14 11:11, Tanay Abhra wrote: diff --git a/config.c b/config.c index a1aef1c..6200f36 100644 --- a/config.c +++ b/config.c @@ -9,6 +9,8 @@ #include exec_cmd.h #include strbuf.h #include quote.h +#include hashmap.h +#include

Re: [PATCH v3 2/3] config: add hashtable for config parsing retrieval

2014-06-23 Thread Junio C Hamano
Tanay Abhra tanay...@gmail.com writes: +static int hashmap_initialized; + ... +static struct hashmap *get_config_cache(void) +{ + static struct hashmap config_cache; + if (!hashmap_initialized) { + config_cache_init(config_cache); + hashmap_initialized = 1;

Re: [PATCH v3 2/3] config: add hashtable for config parsing retrieval

2014-06-23 Thread Junio C Hamano
Ramsay Jones ram...@ramsay1.demon.co.uk writes: +static struct hashmap *get_config_cache(void) +{ +static struct hashmap config_cache; +if (!hashmap_initialized) { +config_cache_init(config_cache); +hashmap_initialized = 1; +