Re: [PATCH] protocol_t upgrade

From: Amos Jeffries <squid3_at_treenet.co.nz>
Date: Wed, 02 Mar 2011 14:48:21 +1300

 On Mon, 28 Feb 2011 21:27:32 -0700, Alex Rousskov wrote:
> On 02/28/2011 07:08 PM, Amos Jeffries wrote:
>> On Mon, 28 Feb 2011 18:37:41 -0700, Alex Rousskov wrote:
>>> On 02/28/2011 04:44 PM, Amos Jeffries wrote:
>>>
>>>>>> +/// Display the Protocol Type (in upper case).
>>>>>> +inline std::ostream &
>>>>>> +operator <<(std::ostream &os, ProtocolType const &p)
>>>>>> +{
>>>>>> + if (p < PROTO_UNKNOWN)
>>>>>> + os << ProtocolType_str[p];
>>>>>> + return os;
>>>>>> +}
>>>>>
>>>>> Should we print something when p is PROTO_UNKNOWN or worse? Where
>>>>> do we
>>>>> use this operator?
>>>>
>>>> Any debugs dumping objects using the type. Potentially any
>>>> streamed
>>>> output of request/reply status lines.
>>>>
>>>> If the protocol is unset or unknown the ProtocolType value is not
>>>> useful. The caller must do the output of any replacement data it
>>>> has.
>>>> Added documentation.
>>>
>>> If the operator is currently used for debugging only, I think we
>>> should
>>> print invalid IDs, to increase the changes we can find the cause of
>>> the
>>> ID-setting bug (and to make the debugging message shape
>>> consistent).
>>>
>>> If the operator is used for message formation, skipping invalid
>>> protocol
>>> IDs would lead to malformed messages, possibly aggravating the
>>> problem,
>>> so an assert() or Must() may be more appropriate, but printing
>>> nothing
>>> is probably worse than printing an ID.
>>>
>>> Not a big deal though and will probably go away (or change) if we
>>> migrate towards an AnyP::Protocol class.
>>
>> Hmm, okay. Fair points.
>>
>> How does this work for you:
>>
>> if (p >= PROTO_NONE && p < PROTO_MAX)
>> os << ProtocolType_str[p];
>> else
>> os << p;
>>
>> Leaving the compiler value in the stream for review when things go
>> bad.
>
> Looks good to me!
>
> The following condition may look more natural to some:
> (PROTO_NONE <= p && p < PROTO_MAX)

 I had to add a cast to prevent infinite inline loop :).

 Attached is an updated patch that I believe clears up all the issues we
 have discussed so far.

 Amos

Received on Wed Mar 02 2011 - 01:48:27 MST

This archive was generated by hypermail 2.2.0 : Wed Mar 02 2011 - 12:00:03 MST