Hi, David: For nodeResultCache.c : +#define SH_EQUAL(tb, a, b) ResultCacheHash_equal(tb, a, b) == 0
I think it would be safer if the comparison is enclosed in parentheses (in case the macro appears in composite condition). +ResultCacheHash_equal(struct resultcache_hash *tb, const ResultCacheKey *key1, + const ResultCacheKey *key2) Since key2 is not used, maybe name it unused_key ? + /* Make a guess at a good size when we're not given a valid size. */ + if (size == 0) + size = 1024; Should the default size be logged ? + /* Update the memory accounting */ + rcstate->mem_used -= freed_mem; Maybe add an assertion that mem_used is >= 0 after the decrement (there is an assertion in remove_cache_entry however, that assertion is after another decrement). + * 'specialkey', if not NULL, causes the function to return false if the entry + * entry which the key belongs to is removed from the cache. duplicate entry (one at the end of first line and one at the beginning of second line). For cache_lookup(), new key is allocated before checking whether rcstate->mem_used > rcstate->mem_upperlimit. It seems new entries should probably have the same size. Can we check whether upper limit is crossed (assuming the addition of new entry) before allocating new entry ? + if (unlikely(!cache_reduce_memory(rcstate, key))) + return NULL; Does the new entry need to be released in the above case? Cheers
