Re: [PATCH] Optimize HttpVersion comparison

On 08/24/2010 07:25 PM, Henrik Nordström wrote:
> tis 2010-08-24 klockan 19:15 -0600 skrev Alex Rousskov:
>
>> 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.
>
> And I say the opposite. The sequence that can fork is sys headers first
> then squid headers with #undef. The keywords are used both in the squid
> header and in squid code.

Note that I was not arguing for one sequence or the other. I was just
answering Amos' question.

> squid heders first then sys headers renders the squid code using these
> members directly broken.

In this particular case, with the compilers we know about, the squid
code using these members directly works fine. This is because the system
headers define a macro with a parameter:

   #define major(foo) gnu_craft_major_device(foo)

The compilers we tested with do not substitute "major" unless it looks
like a function call:

   http_ver.major = 0; // this is fine
   this->major < that.major; // this is fine too
   HttpVersion(): major(0) // this breaks
   HttpVersion(...): major(that.major) // this would break too

This is why the problem is not visible until you try to polish the
HttpVersion class.

However, I would not be surprised if some other compilers behave
differently so, ideally, we should solve the problem for good.

Cheers,

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