<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Aug 24, 2015 at 3:14 AM, Alex Rousskov <span dir="ltr"><<a href="mailto:rousskov@measurement-factory.com" target="_blank">rousskov@measurement-factory.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On 08/23/2015 04:01 PM, Kinkie wrote:<br>
>     Adding comment: "will reallocate if ilen happens to equal pool size"<br>
<br>
</span>I would recommend not adding a comment at all if "no comment" and the<br>
above "low-level description of the current c_str logic" are the two<br>
choices, but it is your call. Not important.<br></blockquote><div><br></div><div>I'm removing the comment; the important thing is that we all agree that this should not add any performance regression in the by-far common case over the code it replaces.<br><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class=""><br>
>     This is what it looks like right now:<br>
><br>
>         //set all items in entries where item is not nullptr and<br>
>         // has same id as argument, to nullptr. Increment count variable<br>
>         // as a side-effect on matches during sweep<br>
>         std::replace_if(entries.begin(), entries.end(),<br>
>             [&](HttpHeaderEntry *e) {<br>
>                 if (e && e->id == id) {<br>
>                     ++count;<br>
>                     return true;<br>
>                 }<br>
>                 return false;<br>
>             },<br>
>             nullptr);<br>
><br>
>     Hopefully the comment will clarify intent :)<br>
<br>
</span>If you think this warrants a comment at all, consider just saying:<br>
<br>
    // replace matching items with nil and count them<br>
<br>
Again, if you think repeating your code using words is better, do that.<br>
Not important.<br></blockquote><div><br></div><div>Using shorter variant.<br></div><div> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
>     example.cc:11:28: error: invalid operands to binary expression<br>
>     ('foo' and 'foo')<br>
>     std::int8_t var = foo::one | foo::four;<br>
>                       ~~~~~~~~ ^ ~~~~~~~~~<br>
>     1 error generated.<br>
<br>
</span>Thank you. We should not mix header IDs and header masks (a mask is not<br>
an ID). I am not saying that you must change how Squid masks headers in<br>
your patch, of course, but it does feel like the compiler has identified<br>
a design flaw in our code. We should fix that at some point. It is your<br>
call whether your patch is the right place for that fix.<br></blockquote><div><br></div><div>It is a common use scenario where we use enums to define bitmasks. There are for sure better alternatives, but this patch is not IMO the right place where to address the issue.<br></div><div> <br></div></div><br>-- <br><div class="gmail_signature">    Francesco</div>
</div></div>