Re: external_acl.cc

On Mon, 23 Jan 2006, Gonzalo Arana wrote:

> Hi,
>
> I'm sory to bother you, but I rather ask you directly instead of
> asking this to the list:

As I inidcated previously I prefer public flaming when I am in error ;-)

> while trying to hunt bug# 1491, I've noticed this in external_acl.cc
> (function externalAclHandleReply()):
> ...
> if (reply)
> entry = external_acl_cache_add(state->def, state->key, entryData);
> else {
> if (reply)
> ** entry = (external_acl_entry *)hash_lookup(state->def->cache, state->key);
> else {
> external_acl_entry *oldentry =
> (external_acl_entry *)hash_lookup(state->def->cache, state->key);
>
> Unless I am reading this bad, the line marked with ** is never
> executed, am I right?

Right..

> This was added by you (unless I am mistaken) while fixing bug# 577, on
> revision 1.35 of external_acl.cc, see:
> http://www.squid-cache.org/cgi-bin/cvsweb.cgi/squid3/src/external_acl.cc.diff?r1=1.35&r2=1.36
>
> I guess that the second 'if (reply)' should be deleted, am I right?

The whole if block should be unwinded it seems. Looks like a bad port of a
Squid-2.5 patch.

The corresponding 2.5 code reads:

     if (cbdataValid(state->def)) {
         if (reply)
             entry = external_acl_cache_add(state->def, state->key, result, user, error);
         else {
             external_acl_entry *oldentry = hash_lookup(state->def->cache, state->key);
             if (oldentry)
                 external_acl_cache_delete(state->def, oldentry);
         }
     }

which makes more sense..

Squid-3 has now been corrected to match this.

Regards
Henrik
Received on Wed Jan 25 2006 - 06:17:20 MST