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

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

 



Ramsay Jones <ramsay@xxxxxxxxxxxxxxxxxxx> 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 config files is the most
important, allowing other users of the config code to use non-callback
functions would be nice too.

Not really for efficiency, but because I find it far more natural to ask
"what's the value of config key XYZ" to the code than to ask "can you
parse all config keys in the file, let me know, and I'll tell you later
which ones I'm interested in".

> [In order to do this, I would have expected to see one hash table
> for each file/blob, so the singleton object took me by surprise.]

The singleton could be fine as long as it is optional. We can have
an API looking like:

int git_config_get_string(const char *key, const char **value) {
	return git_config_get_string_from(the singleton config cache,
                                          key, value);
}

int git_config_get_string_from(struct hashmap *map, const char *key, const char **value);

(or take a structure representing "a set of config files" instead of
directly the hashmap)

Now, the good news is that such API can be written later, without
breaking the API. So I'd say it's fine to keep the code as-is for now,
and make it more general later. No strong opinion, though.

-- 
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]