On Fri, 14 Jul 2006, [EMAIL PROTECTED] wrote:
> +
> +/* Label mapping cache */
> +int cipso_v4_cache_enabled = 1;
> +int cipso_v4_cache_bucketsize = 10;
> +static struct cipso_v4_map_cache_bkt *cipso_v4_cache = NULL;
> +#define CIPSO_V4_CACHE_ENABLED (cipso_v4_cache_enabled && \
> + cipso_v4_cache_bucketsize > 0 && \
> + cipso_v4_cache_size > 0)
This macro seems kind of odd. Why not just check for
cipso_v4_cache_enabled ?
> +static int cipso_v4_cache_init(const u32 bkt_size)
> +{
> + struct cipso_v4_map_cache_bkt *cache;
Why have this temporary local variable,
> + cipso_v4_cache = cache;
then assign its value to a global?
This function is only ever called once, right?
> + * Description:
> + * Clears the CIPSO cache and frees all the memory. This function does not
> + * hold any locks and should only be called when the module is being
> unloaded.
> + *
> + */
> +static int cipso_v4_cache_destroy(void)
> +{
> + struct cipso_v4_map_cache_bkt *cache;
> + struct cipso_v4_map_cache_entry *entry;
> + u32 cache_size;
> + u32 iter;
> +
> + cache = cipso_v4_cache;
> + cache_size = cipso_v4_cache_size;
> + cipso_v4_cache_size = 0;
> + cipso_v4_cache = NULL;
> +
> + for (iter = 0; iter < cache_size; iter++)
> + list_for_each_entry(entry, &cache[iter].list, list) {
> + list_del(&entry->list);
> + cipso_v4_cache_entry_free(entry);
> + }
> +
> + return 0;
> +}
> +
If you're in a permanent cleanup phase, why bother clearing all of these
global variables and using temporary pointers?
Why not just simply free each entry? The way this is coded makes it seem
like you're unsure about the safety of the code.
e.g. what could it possibly matter at this stage whether cipso_v4_cache is
NULL or not?
I think I asked this some time ago: what are the lifetime rules for this
code as a loadable module? What if you arbitrarily rmmod it?
- James
--
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
