Fix some sources of undefined behavior

src/common/Color.cpp

@@ -93,7 +93,7 @@ void StripColors( const char *in, char *out, size_t len )
 		{
 			break;
 		}
-		memcpy( out, text.begin(), text.size() );
+		std::copy( text.begin(), text.end(), out );
 		out += text.size();
 		len -= text.size();
 	}

src/common/Color.h

@@ -102,10 +102,10 @@ class ColorAdaptor<Component*>
 
 	ColorAdaptor( const Component* array ) : array( array ) {}
 
-	Component Red() const { return array[0]; }
-	Component Green() const { return array[1]; }
-	Component Blue() const { return array[2]; }
-	Component Alpha() const { return array[3]; }
+	Component Red() const { return array[ 0 ]; }
+	Component Green() const { return array[ 1 ]; }
+	Component Blue() const { return array[ 2 ]; }
+	Component Alpha() const { return array[ 3 ]; }
 
 private:
 	const Component* array;
@@ -163,7 +163,7 @@ class BasicColor
 	// Initialize from the components
 	CONSTEXPR_FUNCTION BasicColor( component_type r, component_type g, component_type b,
 	       component_type a = component_max ) NOEXCEPT
-		: red( r ), green( g ), blue( b ), alpha( a )
+		: data_{ r, g, b, a }
 	{}
 
     // Default constructor, all components set to zero
@@ -175,85 +175,85 @@ class BasicColor
     BasicColor& operator=( BasicColor&& ) NOEXCEPT = default;
 
 	template<class T, class = std::enable_if<T::is_color>>
-		BasicColor( const T& adaptor ) :
-			red  ( ConvertComponent<T>( adaptor.Red()   ) ),
-			green( ConvertComponent<T>( adaptor.Green() ) ),
-			blue ( ConvertComponent<T>( adaptor.Blue()  ) ),
-			alpha( ConvertComponent<T>( adaptor.Alpha() ) )
+		BasicColor( const T& adaptor ) : data_{
+			ConvertComponent<T>( adaptor.Red() ),
+			ConvertComponent<T>( adaptor.Green() ),
+			ConvertComponent<T>( adaptor.Blue() ),
+			ConvertComponent<T>( adaptor.Alpha() ) }
 		{}
 
 
 	template<class T, class = std::enable_if<T::is_color>>
 		BasicColor& operator=( const T& adaptor )
 		{
-			red   = ConvertComponent<T>( adaptor.Red()   );
-			green = ConvertComponent<T>( adaptor.Green() );
-			blue  = ConvertComponent<T>( adaptor.Blue()  );
-			alpha = ConvertComponent<T>( adaptor.Alpha() );
+			SetRed( ConvertComponent<T>( adaptor.Red() ) );
+			SetGreen( ConvertComponent<T>( adaptor.Green() ) );
+			SetBlue( ConvertComponent<T>( adaptor.Blue() ) );
+			SetAlpha( ConvertComponent<T>( adaptor.Alpha() ) );
 
 			return *this;
 		}
 
 	// Converts to an array
 	CONSTEXPR_FUNCTION const component_type* ToArray() const NOEXCEPT
 	{
-		return &red;
+		return data_;
 	}
 
 	CONSTEXPR_FUNCTION_RELAXED component_type* ToArray() NOEXCEPT
 	{
-		return &red;
+		return data_;
 	}
 
 	void ToArray( component_type* output ) const
 	{
-		memcpy( output, ToArray(), ArrayBytes() );
+		std::copy_n( ToArray(), 4, output );
 	}
 
 	// Size of the memory location returned by ToArray() in bytes
 	CONSTEXPR_FUNCTION std::size_t ArrayBytes() const NOEXCEPT
 	{
-		return 4 * Traits::component_size;
+		return sizeof(data_);
 	}
 
 	CONSTEXPR_FUNCTION component_type Red() const NOEXCEPT
 	{
-		return red;
+		return data_[ 0 ];
 	}
 
 	CONSTEXPR_FUNCTION component_type Green() const NOEXCEPT
 	{
-		return green;
+		return data_[ 1 ];
 	}
 
 	CONSTEXPR_FUNCTION component_type Blue() const NOEXCEPT
 	{
-		return blue;
+		return data_[ 2 ];
 	}
 
 	CONSTEXPR_FUNCTION component_type Alpha() const NOEXCEPT
 	{
-		return alpha;
+		return data_[ 3 ];
 	}
 
 	CONSTEXPR_FUNCTION_RELAXED void SetRed( component_type v ) NOEXCEPT
 	{
-		red = v;
+		data_[ 0 ] = v;
 	}
 
 	CONSTEXPR_FUNCTION_RELAXED void SetGreen( component_type v ) NOEXCEPT
 	{
-		green = v;
+		data_[ 1 ] = v;
 	}
 
 	CONSTEXPR_FUNCTION_RELAXED void SetBlue( component_type v ) NOEXCEPT
 	{
-		blue = v;
+		data_[ 2 ] = v;
 	}
 
 	CONSTEXPR_FUNCTION_RELAXED void SetAlpha( component_type v ) NOEXCEPT
 	{
-		alpha = v;
+		data_[ 3 ] = v;
 	}
 
 	CONSTEXPR_FUNCTION_RELAXED BasicColor& operator*=( float factor ) NOEXCEPT
@@ -262,18 +262,19 @@ class BasicColor
 		return *this;
 	}
 
+	// FIXME: multiplying both rgb AND alpha by something doesn't seem like an operation anyone would want
 	CONSTEXPR_FUNCTION BasicColor operator*( float factor ) const NOEXCEPT
 	{
-		return BasicColor( red * factor, green * factor, blue * factor, alpha * factor );
+		return BasicColor( Red() * factor, Green() * factor, Blue() * factor, Alpha() * factor );
 	}
 
 	// Fits the component values from 0 to component_max
 	CONSTEXPR_FUNCTION_RELAXED void Clamp()
 	{
-		red = Math::Clamp<component_type>( red, component_type(), component_max );
-		green = Math::Clamp<component_type>( green, component_type(), component_max );
-		blue = Math::Clamp<component_type>( blue, component_type(), component_max );
-		alpha = Math::Clamp<component_type>( alpha, component_type(), component_max );
+		SetRed( Math::Clamp<component_type>( Red(), component_type(), component_max ) );
+		SetGreen( Math::Clamp<component_type>( Green(), component_type(), component_max ) );
+		SetBlue( Math::Clamp<component_type>( Blue(), component_type(), component_max ) );
+		SetAlpha( Math::Clamp<component_type>( Alpha(), component_type(), component_max ) );
 	}
 
 private:
@@ -299,8 +300,7 @@ class BasicColor
 		return from;
 	}
 
-	component_type red = 0, green = 0, blue = 0, alpha = 0;
-
+	component_type data_[4]{};
 };
 
 using Color = BasicColor<float>;

src/common/IPC/CommandBuffer.cpp

@@ -134,6 +134,7 @@ namespace IPC {
     }
 
     void CommandBuffer::InternalRead(size_t offset, char* out, size_t len) {
+        ASSERT(out); // must not be null when using memcpy
         char* data = base_ + DATA_OFFSET;
         offset = Normalize(offset);
         size_t canRead = size_ - offset;
@@ -148,6 +149,7 @@ namespace IPC {
     }
 
     void CommandBuffer::InternalWrite(size_t offset, const char* in, size_t len) {
+        ASSERT(in); // must not be null when using memcpy
         char* data = base_ + DATA_OFFSET;
         offset = Normalize(offset);
         size_t canWrite = size_ - offset;

src/common/Serialize.h

@@ -149,6 +149,9 @@ namespace Util {
 
 		void ReadData(void* p, size_t len)
 		{
+			if (!len)
+				return; // ensure null is never passed to memcpy
+
 			if (pos + len <= data.size()) {
 				memcpy(p, data.data() + pos, len);
 				pos += len;

src/common/cm/cm_load.cpp

@@ -840,7 +840,7 @@ CM_ClearMap
 void CM_ClearMap()
 {
 	CM_FreeAll();
-	memset( &cm, 0, sizeof( cm ) );
+	ResetStruct( cm );
 }
 
 /*

src/common/cm/cm_patch.cpp

@@ -668,7 +668,7 @@ static void CM_SurfaceCollideFromGrid( cGrid_t *grid, cSurfaceCollide_t *sc )
 			}
 
 			facet = &facets[ numFacets ];
-			memset( facet, 0, sizeof( *facet ) );
+			*facet = {};
 
 			if ( gridPlanes[ i ][ j ][ 0 ] == gridPlanes[ i ][ j ][ 1 ] )
 			{
@@ -724,7 +724,7 @@ static void CM_SurfaceCollideFromGrid( cGrid_t *grid, cSurfaceCollide_t *sc )
 				}
 
 				facet = &facets[ numFacets ];
-				memset( facet, 0, sizeof( *facet ) );
+				*facet = {};
 
 				facet->surfacePlane = gridPlanes[ i ][ j ][ 1 ];
 				facet->numBorders = 3;
@@ -757,9 +757,9 @@ static void CM_SurfaceCollideFromGrid( cGrid_t *grid, cSurfaceCollide_t *sc )
 	sc->numPlanes = numTempPlanes;
 	sc->numFacets = numFacets;
 	sc->facets = ( cFacet_t * ) CM_Alloc( numFacets * sizeof( *sc->facets ) );
-	memcpy( sc->facets, facets, numFacets * sizeof( *sc->facets ) );
+	std::copy_n( facets, numFacets, sc->facets );
 	sc->planes = ( cPlane_t * ) CM_Alloc( numTempPlanes * sizeof( *sc->planes ) );
-	memcpy( sc->planes, tempPlanes, numTempPlanes * sizeof( *sc->planes ) );
+	std::copy_n( tempPlanes, numTempPlanes, sc->planes );
 }
 
 /*

src/common/cm/cm_trace.cpp

@@ -1846,7 +1846,6 @@ static void CM_Trace( trace_t *results, const vec3_t start, const vec3_t end, co
                       int skipmask, traceType_t type, const sphere_t *sphere )
 {
 	int         i;
-	traceWork_t tw;
 	vec3_t      offset;
 	cmodel_t    *cmod;
 
@@ -1857,7 +1856,7 @@ static void CM_Trace( trace_t *results, const vec3_t start, const vec3_t end, co
 	c_traces++; // for statistics, may be zeroed
 
 	// fill in a default trace
-	memset( &tw, 0, sizeof( tw ) );
+	traceWork_t tw{};
 	tw.trace.fraction = 1; // assume it goes the entire distance until shown otherwise
 	VectorCopy( origin, tw.modelOrigin );
 	tw.type = type;

src/common/cm/cm_trisoup.cpp

@@ -210,7 +210,7 @@ static void CM_SurfaceCollideFromTriangleSoup( cTriangleSoup_t *triSoup, cSurfac
 	for ( i = 0; i < triSoup->numTriangles; i++ )
 	{
 		facet = &facets[ numFacets ];
-		memset( facet, 0, sizeof( *facet ) );
+		*facet = {};
 
 		p1 = triSoup->points[ i ][ 0 ];
 		p2 = triSoup->points[ i ][ 1 ];
@@ -267,11 +267,11 @@ static void CM_SurfaceCollideFromTriangleSoup( cTriangleSoup_t *triSoup, cSurfac
 	// copy the results out
 	sc->numPlanes = numTempPlanes;
 	sc->planes = ( cPlane_t * ) CM_Alloc( numTempPlanes * sizeof( *sc->planes ) );
-	memcpy( sc->planes, tempPlanes, numTempPlanes * sizeof( *sc->planes ) );
+	std::copy_n( tempPlanes, numTempPlanes, sc->planes );
 
 	sc->numFacets = numFacets;
 	sc->facets = ( cFacet_t * ) CM_Alloc( numFacets * sizeof( *sc->facets ) );
-	memcpy( sc->facets, facets, numFacets * sizeof( *sc->facets ) );
+	std::copy_n( facets, numFacets, sc->facets );
 }
 
 /*

src/engine/client/cl_avi.cpp

@@ -331,7 +331,7 @@ bool CL_OpenAVIForWriting( const char *fileName )
 		return false;
 	}
 
-	memset( &afd, 0, sizeof( aviFileData_t ) );
+	afd = {};
 
 	// Don't start if a framerate has not been chosen
 	if ( cl_aviFrameRate->integer <= 0 )

src/engine/client/cl_input.cpp

@@ -696,15 +696,14 @@ CL_CreateCmd
 */
 usercmd_t CL_CreateCmd()
 {
-	usercmd_t cmd;
 	vec3_t    oldAngles;
 
 	VectorCopy( cl.viewangles, oldAngles );
 
 	// keyboard angle adjustment
 	CL_AdjustAngles();
 
-	memset( &cmd, 0, sizeof( cmd ) );
+	usercmd_t cmd{};
 
 	CL_CmdButtons( &cmd );
 
@@ -872,7 +871,6 @@ void CL_WritePacket()
 	byte      data[ MAX_MSGLEN ];
 	int       j;
 	usercmd_t *cmd, *oldcmd;
-	usercmd_t nullcmd;
 	int       packetNum;
 	int       oldPacketNum;
 	int       count;
@@ -883,7 +881,7 @@ void CL_WritePacket()
 		return;
 	}
 
-	memset( &nullcmd, 0, sizeof( nullcmd ) );
+	usercmd_t nullcmd{};
 	oldcmd = &nullcmd;
 
 	MSG_Init( &buf, data, sizeof( data ) );

src/engine/client/cl_main.cpp

@@ -427,8 +427,7 @@ void CL_Record(std::string demo_name)
     }
 
     // baselines
-    entityState_t nullstate;
-    memset( &nullstate, 0, sizeof( nullstate ) );
+    entityState_t nullstate{};
 
     for ( int i = 0; i < MAX_GENTITIES; i++ )
     {
@@ -2292,7 +2291,7 @@ void CL_ShutdownRef()
 	}
 
 	re.Shutdown( true );
-	memset( &re, 0, sizeof( re ) );
+	re = {};
 }
 
 //===========================================================================================

src/engine/client/cl_parse.cpp

@@ -397,7 +397,6 @@ void CL_ParseGamestate( msg_t *msg )
 	int           i;
 	entityState_t *es;
 	int           newnum;
-	entityState_t nullstate;
 	int           cmd;
 
 	Con_Close();
@@ -441,7 +440,7 @@ void CL_ParseGamestate( msg_t *msg )
 				Sys::Drop( "Baseline number out of range: %i", newnum );
 			}
 
-			memset( &nullstate, 0, sizeof( nullstate ) );
+			entityState_t nullstate{};
 			es = &cl.entityBaselines[ newnum ];
 			MSG_ReadDeltaEntity( msg, &nullstate, es, newnum );
 		}