Re: Squid-2.5.STABLE5 still not ready

On Thu, 12 Feb 2004, Duane Wessels wrote:

> This change looks suspicious to me:
>
> @@ -199,11 +199,15 @@
> static void
> ipcacheAddEntry(ipcache_entry * i)
> {
> - hash_link *e = hash_lookup(ip_table, i->hash.key);
> + ipcache_entry *e = (ipcache_entry *) hash_lookup(ip_table, i->hash.key);
> if (NULL != e) {
> - /* avoid colission */
> - ipcache_entry *q = (ipcache_entry *) e;
> - ipcacheRelease(q);
> + /* avoid collision */
> + if (i->flags.negcached && !e->flags.negcached && e->expires > squid_curtime) {
> + /* Don't waste good information */
> + ipcacheFreeEntry(i);
> + return;
> + }
> + ipcacheRelease(e);
> }
> hash_join(ip_table, &i->hash);
> dlinkAdd(i, &i->lru, &lru_list);
>
> Previously we were freeing e (aka q), but now we are freeing i, then inserting
> it into ip_table?

Previously we was replacing e by i. Now we are not in certain conditions
and then intend discarding i instead. Note the return.

But yes, this area is the problem. The new entry (i) is referenced after
return from ipcacheAddEntry() and can not be discarded like this.

Many thanks! Sometimes it needs more than one pair of eyes to see obvious
things.

Regards
Henrik
Received on Thu Feb 12 2004 - 03:14:51 MST