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;

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

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

Reply via email to