[squid-dev] [PATCH] Reduce "!Comm::MonitorsRead(serverConnection->fd)" assertions.

[Amos Jeffries]

On 24/06/17 02:58, Alex Rousskov wrote:
> On 06/23/2017 03:53 AM, Christos Tsantilas wrote:
>> Στις 21/06/2017 08:07 μμ, ο Alex Rousskov έγραψε:
>>>>  >>>> in src/client_side.h:
>>>>
>>>> * I predict we are going to see Coverity complaints about
>>>> PinnedIdleContext missing move semantics.
>>
>> The Coverity should not complain about.
>> Can we test with Coverity before apply the patch, or can we apply and if
>> there are problems implement a move constructor?
> 
> PinnedIdleContext has an implicit move constructor so there is nothing
> to do here IMO. We should not add code unless there are some compelling
> reasons for doing so, and no such reasons have been provided so far in
> this case.
> 
> In general, you can certainly test with Coverity first, but I think that
> requiring such manual tests in this context would be too onerous,
> especially given the current testing setup. Eventually, such pre-commit
> tests of pull requests will be automated.
> 
> 

I have looked up the behaviour I was remembering and it seems it was 
highlighting things that would be an issue with GCC 4 and MSVC 2012 
builds. Those compilers do not implement implicit move constructor or 
operators if _any_ user defined constructor, destructor, or assign 
operators exist.

Given that they are officially not-supported compilers for Squid-4+, I 
think go with as-is and we can work around Coverity later. Though IIRC 
it will still report this as an issue.

Amos