Re: Misc fixes

Stephen R. van den Berg wrote:
 
> Even better. I already was wondering why this was separated.

how about

http_incoming_port port [options]
     address=hostname|ip|"all" (all=0.0.0.0)
     name=XXX (defaults to "port" or "address.port")
     noredirect
     anonymizer=none|default|standard|paranoid

icp_incoming_port port [options]
icp_outgoing_port port [options]
     address=hostname|ip|"all" (all=0.0.0.0)
     name=XXX (defaults to "port" or "address.port")

tcp_outgoing_address
        same as today, for outgoing http/ftp/whatever requests.

and a ACL type to mach HTTP/ICP port name.

Each directive listed once per active port, perhaps with the exception
of icp_outgoing_port for which only one is allowed.

And the redirector interface changed to include the port name (possibly
with a compability option). Maybe the redirector interface should be
redone to a protocol that allows future extensions?

Hmm.. perhaps noredirect and anonymizer should be controlled by ACL
lists, and not part of the port specification.. but my intent is to
allow a single Squid process to serve both filtered and unfiltered
requests. noredirect should most likely be guided by a ACL but it is
more questionable if there is any gain in having a ACL for the
anonymizer.

> >I would use >= here. Not that is matters unless something
> >unexplainable happens..
>
> If something unexplainable happens, we *want* this to crash and burn,
> so that we'll take notice. It would put in a safety net which should
> *not* be catching anything, ever. I don't think this would be good.

Then add a assert call to trap this unexplainable thing (the assert
should probably be in file_map_bit_XXX).

The problem is that when constructs similar to this errors, they tend to
overwrite some other random piece of memory which makes thinks even
worse to debug.

> >same error on storeValidateComplete.
>
> Apparently I'm not using that function in my squid, I've not seen it
> cause problems yet.

I can't tell.. I haven't tried async-io for a while until I saw a report
on squid-bugs that beta24 failed.. I can imagine that
storeValidateComplete may be a problem if clients uses the cache while
Squid is rebuilding..

> This one was a bastard to find, BTW. Threading and debugging don't
> mix so well.

Actually this one was one of the simpler ones I have had with async-io..
It segfaulted right in the failing free() call, and the missing argument
errors was detected by gcc..

> Does your patch add more code than mine? (Just curious; since I
> patched with only minimal knowledge; you must know more about this
> function than I do).

Acutally I did less than you did. All I did was to remove the free call.
The *data pointer here is only there to verify the data structures when
async-io is used. It does not carry any additional information to the
call. The non-async-io implementation passes NULL as data pointer.

> Indeed, squid compiles -Wall-free. I didn't know that. I personally
> don't like some of the -Wall options of gcc, so I rarely use it on
> foreign sources.

-Wall is default for Squid... and should be for any application since
the acceptance of ANSI C in vendor supplied system headers...

---