From d3c92a1ba097b4ebe3ba2b649553b0e07a8aefe2 Mon Sep 17 00:00:00 2001 From: Fletcher Dunn Date: Thu, 9 May 2019 16:19:20 -0700 Subject: [PATCH] Fix CUtlHashMap initialization bug. Fix a pretty sharp edge: when inserting into CUtlHashMap with a function that only takes a key and inserts the default value, we were using "default initialization" ("new T"), which does not initialize PODs. Switch to "value initialization" ("new T{}"). Also some minor compiler compatibility fixes. The version of gcc we're using here didn't like the {}-style call of the base class constructor, so use () instead. *shrug* Also deleted a kludge for an older version of Visual Studio. We were supporting that for a while in Steam, but we've upgraded our min specs and so I'm doing the same here. --- src/public/tier0/platform.h | 11 +++++++++++ src/public/tier1/utlhashmap.h | 10 +++++----- .../clientlib/steamnetworkingsockets_snp.cpp | 7 ------- 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/public/tier0/platform.h b/src/public/tier0/platform.h index 83126ae..f4615fd 100644 --- a/src/public/tier0/platform.h +++ b/src/public/tier0/platform.h @@ -155,6 +155,8 @@ PLATFORM_INTERFACE bool Plat_IsInDebugSession(); // Methods to invoke the constructor, copy constructor, and destructor //----------------------------------------------------------------------------- +// Placement new, using "default initialization". +// THIS DOES NOT INITIALIZE PODS! template inline void Construct( T* pMemory ) { @@ -162,6 +164,15 @@ inline void Construct( T* pMemory ) ::new( pMemory ) T; } +// Placement new, using "value initialization". +// This will zero-initialize PODs +template +inline T* ValueInitializeConstruct( T* pMemory ) +{ + HINT( pMemory != 0 ); + return ::new( pMemory ) T{}; +} + template inline void CopyConstruct( T* pMemory, T const& src ) { diff --git a/src/public/tier1/utlhashmap.h b/src/public/tier1/utlhashmap.h index 4b3b2dd..e66198a 100644 --- a/src/public/tier1/utlhashmap.h +++ b/src/public/tier1/utlhashmap.h @@ -267,7 +267,7 @@ public: Node_t &m_node; const int m_idx; public: - inline ItemRef( const CUtlHashMap< K, T, L, H > &map, int idx ) : m_node{ const_cast< Node_t &>( map.m_memNodes[idx] ) }, m_idx{idx} {} + inline ItemRef( const CUtlHashMap< K, T, L, H > &map, int idx ) : m_node( const_cast< Node_t &>( map.m_memNodes[idx] ) ), m_idx(idx) {} ItemRef( const ItemRef &x ) = default; inline int Index() const { return m_idx; } inline const KeyType_t &Key() { return m_node.m_key; } @@ -275,7 +275,7 @@ public: }; struct MutableItemRef : ItemRef { - inline MutableItemRef( CUtlHashMap< K, T, L, H > &map, int idx ) : ItemRef{ map, idx } {} + inline MutableItemRef( CUtlHashMap< K, T, L, H > &map, int idx ) : ItemRef( map, idx ) {} MutableItemRef( const MutableItemRef &x ) = default; using ItemRef::Element; // const reference inline ElemType_t &Element() const { return this->m_node.m_elem; } // non-const reference @@ -288,7 +288,7 @@ public: CUtlHashMap &m_map; int m_idx; public: - inline Iterator( const CUtlHashMap< K, T, L, H > &map, int idx ) : m_map{ const_cast< CUtlHashMap< K, T, L, H > &>( map ) }, m_idx{idx} {} + inline Iterator( const CUtlHashMap< K, T, L, H > &map, int idx ) : m_map( const_cast< CUtlHashMap< K, T, L, H > &>( map ) ), m_idx(idx) {} Iterator( const Iterator &x ) = default; inline bool operator==( const Iterator &x ) const { return &m_map == &x.m_map && m_idx == x.m_idx; } // Comparing the map reference is probably not necessary in 99% of cases, but needed to be correct inline bool operator!=( const Iterator &x ) const { return &m_map != &x.m_map || m_idx != x.m_idx; } @@ -463,7 +463,7 @@ inline int CUtlHashMap::FindOrInsert( const KeyType_t &key ) int iNodeInserted = InsertUnconstructed( key, &iNodeExisting, false /*no duplicates allowed*/ ); // copies key if ( iNodeInserted != kInvalidIndex ) { - Construct( &m_memNodes[ iNodeInserted ].m_elem ); + ValueInitializeConstruct( &m_memNodes[ iNodeInserted ].m_elem ); return iNodeInserted; } return iNodeExisting; @@ -497,7 +497,7 @@ inline int CUtlHashMap::InsertOrReplace( const KeyType_t &key ) int iNodeInserted = InsertUnconstructed( key, &iNodeExisting, false /*no duplicates allowed*/ ); // copies key if ( iNodeInserted != kInvalidIndex ) { - Construct( &m_memNodes[ iNodeInserted ].m_elem ); + ValueInitializeConstruct( &m_memNodes[ iNodeInserted ].m_elem ); return iNodeInserted; } return iNodeExisting; diff --git a/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_snp.cpp b/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_snp.cpp index 171b324..7d91fa1 100644 --- a/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_snp.cpp +++ b/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_snp.cpp @@ -9,13 +9,6 @@ #include #endif -// Ug -#ifdef _MSC_VER - #if _MSC_VER < 1900 - #define constexpr const - #endif -#endif - // Acks may be delayed. This controls the precision used on the wire to encode the delay time. constexpr int k_nAckDelayPrecisionShift = 5; constexpr SteamNetworkingMicroseconds k_usecAckDelayPrecision = (1 << k_nAckDelayPrecisionShift );