Re: [PATCH] Optimize HttpVersion comparison

From: Alex Rousskov <rousskov_at_measurement-factory.com>
Date: Tue, 24 Aug 2010 19:15:49 -0600

On 08/24/2010 05:57 PM, Amos Jeffries wrote:
> On Tue, 24 Aug 2010 17:21:08 -0600, Alex Rousskov
> <rousskov_at_measurement-factory.com> wrote:
>> On 08/24/2010 04:39 PM, Henrik Nordström wrote:
>>> mån 2010-08-23 klockan 21:44 -0600 skrev Alex Rousskov:
>>>
>>>> I am not going to commit this optimization until there is consensus on
>>>> how to handle the major/minor naming conflict with GCC.
>>>
>>> The #undef should be fine, as long as we do it after including any
>>> system headers which may depend on those macros.
>>
>> Or, if Amos' rules are followed and system headers are always included
>> _after_ Squid ones (the problem would not even exist in this case).
>
> Um, are you saying that header sequence cause the problem? or that it
> resolves it?

The current header sequence (somewhere) violates the squid-then-sys rule
and causes the problem. A header sequence that follows the
squid-then-sys rule will not cause the problem. I suspect such sequence
does not exist (beyond one file scope) because some Squid headers have
to include system headers.

We can find and wrap the offending system headers, moving #undef to the
wrapper header, for a cleaner but more expensive solution.

>>> If not, rename them?
>>>
>>> And perhaps get rid of most if not all direct member accesses isolating
>>> the problem?
>>
>> We could provide accessors, but the good ones will also conflict with
>> the macros. Thus we have two options:
>>
>> 1) #undef
>> 2) rename

> I know its not related directly to the member naming. But this class as a
> whole is used for other protocol beyond HTTP. So should it not be "class
> ProtoVersion"? and moved to the protocol-agnostic location from
> SourceLayout?

Yes, anyp/ProtoVersion or perhaps anyp/MajorMinor to be more specific,
as some protocols probably do not use the Major.minor approach.

Cheers,

Alex.
Received on Wed Aug 25 2010 - 01:15:54 MDT

This archive was generated by hypermail 2.2.0 : Wed Aug 25 2010 - 12:00:05 MDT