[squid-dev] [PATCH] keep track of the size of uploads done with CONNECT

[Amos Jeffries]

On 16/09/2015 7:29 a.m., Aymeric Vincent wrote:
> 
> Hi,
> 
> thank you for your detailed explanations.
> 
> Alex Rousskov <rousskov at measurement-factory.com> writes:
> 
>> 1a. Since you are polishing tunnelStart() profile anyway, delete the
>> status_ptr parameter as well. AFAICT, that parameter is as unneeded as
>> the size_ptr parameter you have already deleted. Use al->http.code
>> inside tunnelStart() instead. This will leave you with two parameters:
>> ClientHttpRequest and AccessLogEntry pointers.
>>
>> 1b. Use (1a) logic above to delete the AccessLogEntry::Pointer parameter
>> as well. Initialize TunnelStateData::al with http->al.
>>
>> 2. Convert TunnelStateData::Connection::size_ptr into an "uint64_t
>> bytesSent" or a similar data member (not a pointer). It will still be
>> updated by TunnelStateData::Connection::dataSent().
>>
>> 3. In TunnelStateData destructor, update ALE "al" as needed, using
>> client.bytesSent and server.bytesSent values. Test that you have guessed
>> which one is which correctly -- they are confusing.
> 
> so I implemented your proposal, except for (2) and (3) because
> TunnelStateData::Connection::size_ptr, I think, should remain for the
> time being. I tried to remove it but switchToTunnel() is using it to
> point to some (other, it seems) http's out.size field due probably to
> the nesting of connections in the ssl bump case. There are other xxx_ptr
> fields in TSD which serve this nesting purpose as well IIUC.
> 
> This means I had to sprinkle a minimal amount of s/int64_t/uint64_t/
> changes to remove the ugly pointer cast in tunnelStart() but they are a
> step in the right direction if I'm not mistaken.
> 

This looks okay in itself. I am just a little suspicious about the
content_length values that could be added to the newly unsigned int
counter variable.

There are quite a number of places in the code like these:
 if (rep->content_length < 0)
 return (r->content_length <= 0 || ...
 if (otherRep->content_length > -1)

Amos