On 18/02/11 18:08, Alex Rousskov wrote:
Code cleanup: Proper assignment and copying of HttpHeader.

Besides being the Right Thing, having correct assignment operator and
copy constructor helps classes that have HttpHeader data members to
avoid defining explicit assignment operators and copy constructors.

Also adds forgotten reset of "len" in the clean() method. Perhaps that
data member is not needed at all?

Other polishing touches. HttpHeader::reset() is now a tiny bit faster.

No runtime changes expected.


Thank you,

Alex.
P.S. I stumbled upon this ancient mess while enhancing adaptation
History classes. This patch will remove a few dozen lines from those
classes, but those changes will be submitted separately.

+1 on this patch going in.


Regarding "len"...

Doxygen shows references by
  HttpHeader::addEntry() - updated, not used.
  HttpHeader::delAt() - updated, not used.
  HttpHeader::insertEntry() - updated, not used.
peerDigestRequest() - just an assert. which is abusing it anyway to check that zero headers are stored.

  HttpRequest::prefixLen() - the only place that actually uses it.

There are only two places that use prefixLen() in turn

 peerDigestFetchSetStats
  - has notes about it being the wrong metric to use.

 clientReplyContext::traceReply
- uses it to set the BODY content length of the trace reply. This appears to be another bug since the request-line and any entity body data are not accounted for. The raw input block from the ConnStateData buffer is what should be sent back right? in which case the size of that can be used without re-packing anything.


So overall yes I believe it should go. But trace and digest needs fixing before that can happen.

Amos
--
Please be using
  Current Stable Squid 2.7.STABLE9 or 3.1.11
  Beta testers wanted for 3.2.0.5

Reply via email to