On 6/24/2014 4:44 AM, Junio C Hamano wrote:
Tanay Abhra <tanayabh@xxxxxxxxx> 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;
I find the arrangement of these two variables somewhat dubious at
the API design level.
If you are going to keep the singleton "config_cache" as a function
scope static, shouldn't the corresponding guard also be in the same
scope?
If you ever need to "uninitialize" to force re-read the file to the
in-core cache, such an uninitializer will need access to not just
the "is hashmap initialized?" boolean (which you do by having it as
a file-scope global like this patch does) but also the thing that
may need to be uninitialized (i.e. the hashmap that may already be
populated), but a function scope static variable config_cache does
not allow access from other places, so you end up calling this
function to initialize it if necessary only to get the pointer to
that structure in order to uninitialize it.
Sounds very twisted and ugly, doesn't it?
Hmn, You are right. but I couldn't find a way for that without making
"config_cache" file-scope static.
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)
+ return;
+ config_cache = get_config_cache();
+ hashmap_iter_init(config_cache, &iter);
+ while ((entry = hashmap_iter_next(&iter))) {
+ free(entry->key);
+ string_list_clear(&entry->value_list, 1);
+ }
+ hashmap_free(config_cache, 1);
+ hashmap_initialized = 0;
+}
+
The idea was only initialize the hashmap only once,
as it is fed by git_config(). I ended up following
this approach where the map would be lazily generated whenever
a function asked for the map. Is there another way out?
--
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