]> git.ipfire.org Git - thirdparty/squid.git/commit - src/Transients.cc
Maintenance: Consistent use of C++11 "override" specifier (#1224)
authorAlex Rousskov <rousskov@measurement-factory.com>
Mon, 9 Jan 2023 21:12:02 +0000 (21:12 +0000)
committerSquid Anubis <squid-anubis@squid-cache.org>
Fri, 13 Jan 2023 19:24:42 +0000 (19:24 +0000)
commit337b9aa4786047dfd90bac5ebf0ac4b4a475fc03
tree29b4c8b847ac7f4c2f367eec8a6d363141bd7ab5
parent27c36771bf145c2f8ca1efab6743b9e087867ab5
Maintenance: Consistent use of C++11 "override" specifier (#1224)

Start following CppCoreGuidelines recommendation C.128: Virtual
functions should specify exactly one of virtual, override, or final.

Used clang-tidy modernize-use-override check to mark overridden methods.
Clang-tidy uses clang AST, so we can only modernize code that clang can
see, but we can enable enough ./configure options for clang to reach
most (possibly all) relevant classes in a single build.

Manual gradual conversion of modified (for other reasons) method
declarations is impractical because some compilers complain when a class
uses an inconsistent _mix_ of virtual and override declarations:

    warning: 'toCbdata' overrides a member function but is not marked
    'override' [-Winconsistent-missing-override]

### cbdata changes

The following manual changes avoid -Winconsistent-missing-override
warnings. Doing these separately, either before or after applying
modernize-use-override, would trigger those warnings.

* Replaced CbdataParent-CBDATA_CLASS with CbdataParent-CBDATA_CHILD
  class inheritance sequences. This fix does not really change anything
  except for making toCbdata() specifiers consistent.

* Replaced CbdataParent-CBDATA_CLASS-CBDATA_CHILD with
  CbdataParent-CBDATA_INTERMEDIATE-CBDATA_CHILD class inheritance
  sequences. This fix removes unused new/delete operators in
  intermediate classes (and associated unused static memory pools) and
  makes toCbdata() specifiers consistent. This fix was difficult to get
  right because of the old design problem discussed below.

While working on the second bullet changes, we first tried to simply
drop the intermediate CBDATA_CLASS sequence element because it should
never be needed/used in class hierarchies ending with CBDATA_CHILD.
Fortunately, CI tests discovered that dropping CBDATA_CLASS converts an
old design problem into runtime bugs: Those intermediate class
constructors (e.g., Mgr::StoreToCommWriter) actually call toCbdata()
when creating connection closure callbacks and such!

During intermediate constructor execution, the class virtual table does
not yet point to toCbdata() defined by CBDATA_CHILD. If we simply remove
CBDATA_CLASS, then, during that intermediate constructor execution, the
virtual table would still point to pure CbdataParent::toCbdata(). Those
intermediate constructors would then crash Squid:

    FATAL: Dying from an exception handling failure;
    exception: [no active exception]

    in OnTerminate () at main.cc:1310
    in std::terminate() () from libstdc++.so.6
    in __cxa_pure_virtual () from libstdc++.so.6
    in CbcPointer<Mgr::StoreToCommWriter>::CbcPointer at CbcPointer.h:94
    in Mgr::StoreToCommWriter::StoreToCommWriter at ...oCommWriter.cc:29

We now use lighter CBDATA_INTERMEDIATE() instead of intermediate
CBDATA_CLASS. The resulting code is as risky as it used to be, but at
least we are marking this (old) design problem (in some cases).

We rejected the idea of defining CbdataParent::toCbdata() instead. It
would work fine for linear class inheritance where "this" does not
change -- in those simple class hierarchies, we do not need toCbdata()!
However, such a change would make any constructor-time bugs in multiple
inheritance hierarchies much more difficult to find because the faulty
constructor will work instead of triggering the above FATAL error. The
crashes would happen later, when wrong cbdata pointer is used. While
CBDATA_INTERMEDIATE() cannot fix this design problem, it does not make
it _worse_, so we prefer that solution.
349 files changed:
lib/libTrie/TrieCharTransform.h
src/AccessLogEntry.h
src/BodyPipe.cc
src/BodyPipe.h
src/ClientDelayConfig.h
src/ClientInfo.h
src/ClientRequestContext.h
src/CollapsedForwarding.cc
src/CommCalls.h
src/CompositePoolNode.h
src/ConfigOption.h
src/DelayIdComposite.h
src/DelayTagged.h
src/DelayUser.h
src/DelayVector.h
src/DiskIO/AIO/AIODiskFile.h
src/DiskIO/AIO/AIODiskIOModule.h
src/DiskIO/AIO/AIODiskIOStrategy.h
src/DiskIO/Blocking/BlockingDiskIOModule.h
src/DiskIO/Blocking/BlockingFile.h
src/DiskIO/Blocking/BlockingIOStrategy.h
src/DiskIO/DiskDaemon/DiskDaemonDiskIOModule.h
src/DiskIO/DiskDaemon/DiskdAction.h
src/DiskIO/DiskDaemon/DiskdFile.h
src/DiskIO/DiskDaemon/DiskdIOStrategy.h
src/DiskIO/DiskIOStrategy.h
src/DiskIO/DiskThreads/DiskThreadsDiskFile.h
src/DiskIO/DiskThreads/DiskThreadsDiskIOModule.h
src/DiskIO/DiskThreads/DiskThreadsIOStrategy.h
src/DiskIO/IpcIo/IpcIoDiskIOModule.h
src/DiskIO/IpcIo/IpcIoFile.cc
src/DiskIO/IpcIo/IpcIoFile.h
src/DiskIO/IpcIo/IpcIoIOStrategy.h
src/DiskIO/Mmapped/MmappedDiskIOModule.h
src/DiskIO/Mmapped/MmappedFile.h
src/DiskIO/Mmapped/MmappedIOStrategy.h
src/DiskIO/ReadRequest.h
src/DiskIO/WriteRequest.h
src/Downloader.cc
src/Downloader.h
src/ExternalACL.h
src/ExternalACLEntry.h
src/FwdState.h
src/HappyConnOpener.cc
src/HappyConnOpener.h
src/HttpReply.h
src/HttpRequest.h
src/ICP.h
src/MemBuf.h
src/MemStore.cc
src/MemStore.h
src/MessageBucket.h
src/MessageDelayPools.h
src/Notes.h
src/NullDelayId.h
src/PeerPoolMgr.cc
src/PeerPoolMgr.h
src/PeerSelectState.h
src/SBufStatsAction.h
src/Store.h
src/StoreClient.h
src/StoreIOState.h
src/StoreSearch.h
src/Transients.cc
src/Transients.h
src/acl/AdaptationService.h
src/acl/AdaptationServiceData.h
src/acl/AllOf.h
src/acl/AnnotateClient.h
src/acl/AnnotateTransaction.h
src/acl/AnnotationData.h
src/acl/AnyOf.h
src/acl/Arp.h
src/acl/Asn.h
src/acl/AtStep.h
src/acl/AtStepData.h
src/acl/BoolOps.h
src/acl/Certificate.h
src/acl/CertificateData.h
src/acl/Checklist.h
src/acl/ConnMark.h
src/acl/ConnectionsEncrypted.h
src/acl/DestinationAsn.h
src/acl/DestinationDomain.h
src/acl/DestinationIp.h
src/acl/DomainData.h
src/acl/Eui64.h
src/acl/ExtUser.h
src/acl/FilledChecklist.h
src/acl/HasComponent.h
src/acl/HasComponentData.h
src/acl/HierCode.h
src/acl/HierCodeData.h
src/acl/HttpHeaderData.h
src/acl/HttpRepHeader.h
src/acl/HttpReqHeader.h
src/acl/HttpStatus.h
src/acl/InnerNode.h
src/acl/IntRange.h
src/acl/Ip.h
src/acl/LocalIp.h
src/acl/LocalPort.h
src/acl/MaxConnection.h
src/acl/Method.h
src/acl/MethodData.h
src/acl/MyPortName.h
src/acl/Note.h
src/acl/NoteData.h
src/acl/Options.h
src/acl/PeerName.h
src/acl/Protocol.h
src/acl/ProtocolData.h
src/acl/Random.h
src/acl/RegexData.h
src/acl/ReplyHeaderStrategy.h
src/acl/RequestHeaderStrategy.h
src/acl/ServerCertificate.h
src/acl/ServerName.h
src/acl/SourceAsn.h
src/acl/SourceDomain.h
src/acl/SourceIp.h
src/acl/SquidError.h
src/acl/SquidErrorData.h
src/acl/SslError.h
src/acl/SslErrorData.h
src/acl/Strategised.h
src/acl/StringData.h
src/acl/Tag.h
src/acl/Time.h
src/acl/TimeData.h
src/acl/TransactionInitiator.h
src/acl/Tree.h
src/acl/Url.h
src/acl/UrlLogin.h
src/acl/UrlPath.h
src/acl/UrlPort.h
src/acl/UserData.h
src/adaptation/AccessCheck.h
src/adaptation/Initiate.cc
src/adaptation/Initiate.h
src/adaptation/Initiator.h
src/adaptation/Iterator.h
src/adaptation/Service.h
src/adaptation/ServiceGroups.h
src/adaptation/ecap/Config.h
src/adaptation/ecap/Host.h
src/adaptation/ecap/MessageRep.h
src/adaptation/ecap/ServiceRep.cc
src/adaptation/ecap/ServiceRep.h
src/adaptation/ecap/XactionRep.cc
src/adaptation/ecap/XactionRep.h
src/adaptation/icap/Config.h
src/adaptation/icap/Launcher.h
src/adaptation/icap/ModXact.h
src/adaptation/icap/OptXact.h
src/adaptation/icap/ServiceRep.h
src/adaptation/icap/Xaction.cc
src/adaptation/icap/Xaction.h
src/anyp/PortCfg.h
src/auth/AclMaxUserIp.h
src/auth/AclProxyAuth.h
src/auth/CredentialsCache.cc
src/auth/Scheme.h
src/auth/User.h
src/auth/UserRequest.h
src/auth/basic/Config.h
src/auth/basic/Scheme.h
src/auth/basic/User.h
src/auth/basic/UserRequest.h
src/auth/digest/Config.h
src/auth/digest/Scheme.h
src/auth/digest/User.h
src/auth/digest/UserRequest.h
src/auth/negotiate/Config.h
src/auth/negotiate/Scheme.h
src/auth/negotiate/User.h
src/auth/negotiate/UserRequest.h
src/auth/ntlm/Config.h
src/auth/ntlm/Scheme.cc
src/auth/ntlm/Scheme.h
src/auth/ntlm/User.h
src/auth/ntlm/UserRequest.h
src/base/AsyncCall.h
src/base/AsyncCallbacks.h
src/base/AsyncCbdataCalls.h
src/base/AsyncFunCalls.h
src/base/AsyncJob.h
src/base/AsyncJobCalls.h
src/base/CodeContext.cc
src/base/CodeContext.h
src/base/PackableStream.h
src/base/RunnersRegistry.h
src/base/Subscription.h
src/base/TextException.h
src/cache_cf.cc
src/cache_manager.cc
src/cbdata.h
src/client_db.cc
src/client_side.cc
src/client_side.h
src/client_side_reply.h
src/client_side_request.h
src/clients/Client.h
src/clients/FtpClient.h
src/clients/FtpGateway.cc
src/clients/FtpRelay.cc
src/clients/HttpTunneler.h
src/comm.h
src/comm/ConnOpener.h
src/comm/Connection.h
src/comm/TcpAcceptor.h
src/debug/debug.cc
src/delay_pools.cc
src/dns_internal.cc
src/error/Detail.cc
src/error/Detail.h
src/error/ExceptionErrorDetail.h
src/error/SysErrorDetail.h
src/errorpage.cc
src/esi/Assign.h
src/esi/Context.h
src/esi/Element.h
src/esi/Esi.cc
src/esi/ExpatParser.cc
src/esi/ExpatParser.h
src/esi/Include.h
src/esi/Libxml2Parser.cc
src/esi/Libxml2Parser.h
src/esi/Literal.h
src/esi/Segment.h
src/esi/Sequence.h
src/esi/VarState.h
src/event.cc
src/event.h
src/fs/rock/RockHeaderUpdater.h
src/fs/rock/RockIoState.cc
src/fs/rock/RockIoState.h
src/fs/rock/RockRebuild.h
src/fs/rock/RockStoreFileSystem.h
src/fs/rock/RockSwapDir.h
src/fs/ufs/StoreFSufs.h
src/fs/ufs/StoreSearchUFS.h
src/fs/ufs/UFSStoreState.h
src/fs/ufs/UFSSwapDir.cc
src/fs/ufs/UFSSwapDir.h
src/fs/ufs/UFSSwapLogParser.cc
src/helper.h
src/htcp.cc
src/http.h
src/http/Message.h
src/http/Stream.h
src/http/one/Parser.h
src/http/one/RequestParser.h
src/http/one/ResponseParser.h
src/http/one/TeChunkedParser.h
src/icmp/Icmp4.h
src/icmp/Icmp6.h
src/icmp/IcmpPinger.h
src/icmp/IcmpSquid.h
src/icp_v2.cc
src/icp_v3.cc
src/ident/AclIdent.h
src/ipc/Coordinator.h
src/ipc/Forwarder.cc
src/ipc/Forwarder.h
src/ipc/Inquirer.cc
src/ipc/Inquirer.h
src/ipc/Port.h
src/ipc/Queue.h
src/ipc/Strand.h
src/ipc/UdsOp.h
src/ipc/mem/Pages.cc
src/ipc/mem/Segment.h
src/ipcache.h
src/log/TcpLogger.h
src/main.cc
src/mem/Pool.h
src/mem/PoolChunked.h
src/mem/PoolMalloc.h
src/mgr/Action.h
src/mgr/ActionCreator.h
src/mgr/ActionWriter.h
src/mgr/BasicActions.h
src/mgr/CountersAction.h
src/mgr/Filler.h
src/mgr/Forwarder.h
src/mgr/FunAction.h
src/mgr/InfoAction.h
src/mgr/Inquirer.h
src/mgr/IntParam.h
src/mgr/IntervalAction.h
src/mgr/IoAction.h
src/mgr/QueryParam.h
src/mgr/Request.h
src/mgr/Response.h
src/mgr/ServiceTimesAction.h
src/mgr/StoreIoAction.h
src/mgr/StoreToCommWriter.cc
src/mgr/StoreToCommWriter.h
src/mgr/StringParam.h
src/mime.cc
src/pconn.h
src/sbuf/MemBlob.h
src/security/BlindPeerConnector.h
src/security/ErrorDetail.h
src/security/PeerConnector.cc
src/security/PeerConnector.h
src/security/ServerOptions.h
src/security/Session.cc
src/servers/FtpServer.h
src/servers/Http1Server.h
src/servers/Server.h
src/snmp/Forwarder.h
src/snmp/Inquirer.h
src/snmp/Request.h
src/snmp/Response.h
src/snmp_core.h
src/ssl/ErrorDetailManager.cc
src/ssl/PeekingPeerConnector.h
src/ssl/bio.h
src/ssl/context_storage.h
src/store/Controller.h
src/store/Disk.h
src/store/Disks.h
src/store/LocalSearch.cc
src/store/Storage.h
src/tests/CapturingStoreEntry.h
src/tests/TestSwapDir.h
src/tests/stub_ipc_Forwarder.cc
src/tests/stub_libsecurity.cc
src/tests/testACLMaxUserIP.h
src/tests/testCacheManager.h
src/tests/testConfigParser.h
src/tests/testDiskIO.h
src/tests/testEvent.h
src/tests/testEventLoop.cc
src/tests/testHttpReply.h
src/tests/testHttpRequest.h
src/tests/testIcmp.h
src/tests/testPackableStream.h
src/tests/testRefCount.cc
src/tests/testRock.h
src/tests/testStore.h
src/tests/testString.h
src/tests/testURL.h
src/tests/testUriScheme.h
src/tunnel.cc
src/urn.cc
test-suite/VirtualDeleteOperator.cc