[squid-dev] [PATCH] Coverity(-inspired) fixes part four, HttpHeader refactor

[Amos Jeffries]

On 24/08/2015 1:14 p.m., Alex Rousskov wrote:
> On 08/23/2015 04:01 PM, Kinkie wrote:
>>     Adding comment: "will reallocate if ilen happens to equal pool size"
> 
> I would recommend not adding a comment at all if "no comment" and the
> above "low-level description of the current c_str logic" are the two
> choices, but it is your call. Not important.

Agreed.

Which is also why I just suggested the terse "performance regression,
c_str() reallocates" in all cases where it *can*. Removal of those cases
is still important (thus X vs TODO) but not reasonable at current time.


> 
>>     example.cc:11:28: error: invalid operands to binary expression
>>     ('foo' and 'foo')
>>     std::int8_t var = foo::one | foo::four;
>>                       ~~~~~~~~ ^ ~~~~~~~~~
>>     1 error generated.
> 
> Thank you. We should not mix header IDs and header masks (a mask is not
> an ID). I am not saying that you must change how Squid masks headers in
> your patch, of course, but it does feel like the compiler has identified
> a design flaw in our code. We should fix that at some point. It is your
> call whether your patch is the right place for that fix.

This is not about header IDs - thats a different alteration leading to
this change. This bitmask thing is about enumerating the property flags
that can be set in the table record. So we dont have to list false
explicitly for all the irrelevant ones, or to depend on ordering.


The design flaw I think is in use of enum class at all. Its just not
suitable for bitmasks since the OR/AND-ing individual mask values does
not produce a valid point-value within the defined set of enum class
point-values. Wantig the compiler to reject that is exactly when enum
class should be used.

The basic c-style enum provides implicit type-casting to int_type which
is what we need when using an enum to name bitmask bits. It does not
necessarily mean badness. Just use with care. Its bad reputation is from
people who didn't, or were forced to abuse it.

Amos