engine: cvar: implement safer cvar unlinking in case when game deallocates cvar

engine/common/cvar.c

@@ -272,6 +272,15 @@ static qboolean Cvar_ValidateVarName( const char *s, qboolean isvalue )
 	return true;
 }
 
+static void Cvar_Free( convar_t *var )
+{
+	freestring( var->name );
+	freestring( var->string );
+	freestring( var->def_string );
+	freestring( var->desc );
+	Mem_Free( var );
+}
+
 /*
 ============
 Cvar_UnlinkVar
@@ -311,17 +320,13 @@ static int Cvar_UnlinkVar( const char *var_name, int group )
 #endif
 
 		// unlink variable from list
-		freestring( var->string );
 		*prev = var->next;
 
 		// only allocated cvars can throw these fields
 		if( FBitSet( var->flags, FCVAR_ALLOCATED ))
-		{
+			Cvar_Free( var );
-			freestring( var->name );
+		else
-			freestring( var->def_string );
+			freestring( var->string );
-			freestring( var->desc );
-			Mem_Free( var );
-		}
 		count++;
 	}
 
@@ -1217,6 +1222,20 @@ static void Cvar_List_f( void )
 	Con_Printf( "\n%i cvars\n", count );
 }
 
+static qboolean Cvar_ValidateUnlinkGroup( int group )
+{
+	if( FBitSet( group, FCVAR_EXTDLL ) && !Cvar_VariableInteger( "host_gameloaded" ))
+		return false;
+
+	if( FBitSet( group, FCVAR_CLIENTDLL ) && !Cvar_VariableInteger( "host_clientloaded" ))
+		return false;
+
+	if( FBitSet( group, FCVAR_GAMEUIDLL ) && !Cvar_VariableInteger( "host_gameuiloaded" ))
+		return false;
+
+	return true;
+}
+
 /*
 ============
 Cvar_Unlink
@@ -1228,19 +1247,95 @@ void Cvar_Unlink( int group )
 {
 	int	count;
 
-	if( Cvar_VariableInteger( "host_gameloaded" ) && FBitSet( group, FCVAR_EXTDLL ))
+	if( !Cvar_ValidateUnlinkGroup( group ))
-		return;
-
-	if( Cvar_VariableInteger( "host_clientloaded" ) && FBitSet( group, FCVAR_CLIENTDLL ))
-		return;
-
-	if( Cvar_VariableInteger( "host_gameuiloaded" ) && FBitSet( group, FCVAR_GAMEUIDLL ))
 		return;
 
 	count = Cvar_UnlinkVar( NULL, group );
 	Con_Reportf( "unlink %i cvars\n", count );
 }
 
+pending_cvar_t *Cvar_PrepareToUnlink( int group )
+{
+	pending_cvar_t *list = NULL;
+	pending_cvar_t *tail = NULL;
+	convar_t *cv;
+
+	for( cv = cvar_vars; cv != NULL; cv = cv->next )
+	{
+		size_t namelen;
+		pending_cvar_t *p;
+
+		if( !FBitSet( cv->flags, group ))
+			continue;
+
+		namelen = Q_strlen( cv->name ) + 1;
+		p = Mem_Malloc( host.mempool, sizeof( *list ) + namelen );
+		p->next = NULL;
+		p->cv_cur = cv;
+		p->cv_next = cv->next;
+		p->cv_allocated = FBitSet( cv->flags, FCVAR_ALLOCATED ) ? true : false;
+		Q_strncpy( p->cv_name, cv->name, namelen );
+
+		if( list == NULL )
+			list = p;
+		else
+			tail->next = p;
+
+		tail = p;
+	}
+
+	return list;
+}
+
+void Cvar_UnlinkPendingCvars( pending_cvar_t *list )
+{
+	int count = 0;
+
+	while( list != NULL )
+	{
+		pending_cvar_t *next = list->next;
+		convar_t *cv_prev, *cv;
+
+		for( cv_prev = NULL, cv = cvar_vars; cv != NULL; cv_prev = cv, cv = cv->next )
+		{
+			if( cv == list->cv_cur )
+				break;
+		}
+
+		if( cv == NULL )
+		{
+			Con_Reportf( "%s: can't find %s in variable list\n", __func__, list->cv_name );
+			Mem_Free( list );
+			list = next;
+			continue;
+		}
+
+		// unlink cvar from list
+		BaseCmd_Remove( HM_CVAR, list->cv_name );
+		if( cv_prev != NULL )
+			cv_prev->next = list->cv_next;
+		else cvar_vars = list->cv_next;
+
+		if( list->cv_allocated )
+			Cvar_Free( list->cv_cur );
+		else
+		{
+			// TODO: can't free cvar string here because
+			// it's not safe to access cv_cur and
+			// can't save string pointer because it could've been changed
+			// and pointer to it is already lost
+			// freestring( list->cv_string );
+		}
+
+		// now free pending cvar
+		Mem_Free( list );
+		list = next;
+		count++;
+	}
+
+	Con_Reportf( "unlink %i cvars\n", count );
+}
+
 /*
 ============
 Cvar_Init

engine/common/cvar.h

@@ -18,6 +18,18 @@ GNU General Public License for more details.
 
 #include "cvardef.h"
 
+// As some mods dynamically allocate cvars and free them without notifying the engine
+// let's construct a list of cvars that must be removed
+typedef struct pending_cvar_s
+{
+	struct pending_cvar_s *next;
+
+	convar_t *cv_cur; // preserve the data that might get freed
+	convar_t *cv_next;
+	qboolean  cv_allocated; // if it's allocated by us, it's safe to access cv_cur
+	char      cv_name[];
+} pending_cvar_t;
+
 cvar_t *Cvar_GetList( void );
 #define Cvar_FindVar( name )	Cvar_FindVarExt( name, 0 )
 convar_t *Cvar_FindVarExt( const char *var_name, int ignore_group );
@@ -43,4 +55,7 @@ void Cvar_Init( void );
 void Cvar_PostFSInit( void );
 void Cvar_Unlink( int group );
 
+pending_cvar_t *Cvar_PrepareToUnlink( int group );
+void Cvar_UnlinkPendingCvars( pending_cvar_t *pending_cvars );
+
 #endif//CVAR_H