Properly close file handles in the case of errors

Razish · Dec 28, 2017 · 3330bf1 · 3330bf1
1 parent 3e4b371
commit 3330bf1
Show file tree

Hide file tree

Showing 17 changed files with 57 additions and 21 deletions.
diff --git a/cgame/cg_main.cpp b/cgame/cg_main.cpp
@@ -1289,8 +1289,8 @@ void CG_LoadMenus( const char *menuFile ) {
 	}
 
 	if ( len >= MAX_MENUDEFFILE ) {
-		trap->Error( ERR_DROP, S_COLOR_RED "menu file too large: %s is %i, max allowed is %i", menuFile, len, MAX_MENUDEFFILE );
 		trap->FS_Close( f );
+		trap->Error( ERR_DROP, S_COLOR_RED "menu file too large: %s is %i, max allowed is %i", menuFile, len, MAX_MENUDEFFILE );
 		return;
 	}
 

diff --git a/cgame/cg_players.cpp b/cgame/cg_players.cpp
@@ -270,6 +270,7 @@ qboolean CG_ParseSurfsFile( const char *modelName, const char *skinName, char *s
 		return qfalse;
 	if ( len >= sizeof(text)-1 ) {
 		Com_Printf( "File %s too long\n", sfilename );
+		trap->FS_Close( f );
 		return qfalse;
 	}
 

diff --git a/cgame/cg_saga.cpp b/cgame/cg_saga.cpp
@@ -128,7 +128,11 @@ void CG_InitSiegeMode( void ) {
 	Com_sprintf( levelname, sizeof(levelname), "maps/%s.siege", cgs.mapnameClean );
 
 	len = trap->FS_Open( levelname, &f, FS_READ );
-	if ( !f || len <= 0 || len >= MAX_SIEGE_INFO_SIZE ) {
+	if ( !f || len <= 0 ) {
+		goto failure;
+	}
+	if ( len >= MAX_SIEGE_INFO_SIZE ) {
+		trap->FS_Close( f );
 		goto failure;
 	}
 

diff --git a/game/ai_util.cpp b/game/ai_util.cpp
@@ -561,6 +561,7 @@ void BotUtilizePersonality( bot_state_t *bs ) {
 	if ( len >= BUFSIZE ) {
 		trap->Print( S_COLOR_RED "Personality file exceeds maximum length\n" );
 		B_TempFree( BUFSIZE ); //buf
+		trap->FS_Close( f );
 		return;
 	}
 

diff --git a/game/ai_wpnav.cpp b/game/ai_wpnav.cpp
@@ -1577,6 +1577,7 @@ int LoadPathData( const char *filename ) {
 
 	if ( len >= 524288 ) {
 		trap->Print( S_COLOR_RED "Route file exceeds maximum length\n" );
+		trap->FS_Close( f );
 		return 0;
 	}
 

diff --git a/game/bg_misc.cpp b/game/bg_misc.cpp
@@ -326,10 +326,10 @@ const stringID_table_t eTypes[ET_MAX] = {
 
 static qboolean BG_FileExists( const char *fileName ) {
 	if ( fileName && fileName[0] ) {
-		int fh = 0;
-		trap->FS_Open( fileName, &fh, FS_READ );
-		if ( fh > 0 ) {
-			trap->FS_Close( fh );
+		fileHandle_t f = NULL_FILE;
+		trap->FS_Open( fileName, &f, FS_READ );
+		if ( f ) {
+			trap->FS_Close( f );
 			return qtrue;
 		}
 	}

diff --git a/game/bg_panimate.cpp b/game/bg_panimate.cpp
@@ -2090,15 +2090,16 @@ int BG_ParseAnimationFile( const char *filename, animation_t *animset, qboolean
 	// load the file
 	if ( !BGPAFtextLoaded || !isHumanoid ) { //rww - We are always using the same animation config now. So only load it once.
 		len = trap->FS_Open( filename, &f, FS_READ );
-		if ( (len <= 0) || (len >= sizeof(BGPAFtext)-1) ) {
+		if ( !f || len <= 0 ) {
 			if ( dynAlloc ) {
 				BG_AnimsetFree( animset );
 			}
-			if ( len > 0 ) {
-				Com_Error( ERR_DROP, "%s exceeds the allowed game-side animation buffer!", filename );
-			}
 			return -1;
 		}
+		if ( len >= sizeof(BGPAFtext)-1 ) {
+			trap->FS_Close( f );
+			Com_Error( ERR_DROP, "%s exceeds the allowed game-side animation buffer!", filename );
+		}
 
 		trap->FS_Read( BGPAFtext, len, f );
 		BGPAFtext[len] = 0;

diff --git a/game/bg_saberLoad.cpp b/game/bg_saberLoad.cpp
@@ -2230,6 +2230,7 @@ void WP_SaberLoadParms( void ) {
 		}
 
 		if ( (totallen + len + 1) >= MAX_SABER_DATA_SIZE ) {
+			trap->FS_Close( f );
 #ifdef PROJECT_UI
 			Com_Error( ERR_FATAL, "WP_SaberLoadParms: Saber extensions (*.sab) are too large!\nRan out of space before reading %s", holdChar );
 #else

diff --git a/game/bg_saga.cpp b/game/bg_saga.cpp
@@ -626,7 +626,11 @@ void BG_SiegeParseClassFile( const char *filename, siegeClassDesc_t *descBuffer
 
 	len = trap->FS_Open( filename, &f, FS_READ );
 
-	if ( !f || len >= 4096 ) {
+	if ( !f ) {
+		return;
+	}
+	if ( len >= 4096 ) {
+		trap->FS_Close( f );
 		return;
 	}
 
@@ -1020,7 +1024,11 @@ void BG_SiegeParseTeamFile( const char *filename ) {
 
 	len = trap->FS_Open( filename, &f, FS_READ );
 
-	if ( !f || len >= 2048 ) {
+	if ( !f ) {
+		return;
+	}
+	if ( len >= 2048 ) {
+		trap->FS_Close( f );
 		return;
 	}
 

diff --git a/game/bg_vehicleLoad.cpp b/game/bg_vehicleLoad.cpp
@@ -1179,6 +1179,7 @@ void BG_VehWeaponLoadParms( void ) {
 			}
 
 			if ( totallen + len >= MAX_VEH_WEAPON_DATA_SIZE ) {
+				trap->FS_Close( f );
 				Com_Error( ERR_DROP, "Vehicle Weapon extensions (*.vwp) are too large" );
 			}
 			strcat( marker, tempReadBuffer );
@@ -1240,6 +1241,7 @@ void BG_VehicleLoadParms( void ) {//HMM... only do this if there's a vehicle on
 			}
 
 			if ( totallen + len >= MAX_VEHICLE_DATA_SIZE ) {
+				trap->FS_Close( f );
 				Com_Error( ERR_DROP, "Vehicle extensions (*.veh) are too large" );
 			}
 			strcat( marker, tempReadBuffer );

diff --git a/game/g_admin.cpp b/game/g_admin.cpp
@@ -346,7 +346,7 @@ static void AM_DrawString( int type, gentity_t *ent, const char *arg, char *arg2
 
 void AM_LoadStrings( void ) {
 	fileHandle_t f = NULL_FILE;
-	unsigned int len = trap->FS_Open( "admin_strings.json", &f, FS_READ );
+	int len = trap->FS_Open( "admin_strings.json", &f, FS_READ );
 	trap->Print( "Loading admin strings\n" );
 
 	// no file
@@ -365,6 +365,7 @@ void AM_LoadStrings( void ) {
 
 	char *buf = NULL;
 	if ( !(buf = (char *)malloc( len + 1 )) ) {
+		trap->FS_Close( f );
 		return;
 	}
 
@@ -595,6 +596,7 @@ void AM_LoadAdmins( void ) {
 
 	// alloc memory for buffer
 	if ( !(buf = (char*)malloc( len + 1 )) ) {
+		trap->FS_Close( f );
 		return;
 	}
 
@@ -896,6 +898,7 @@ void AM_LoadTelemarks( void ) {
 
 	// alloc memory for buffer
 	if ( !(buf = (char*)malloc( len + 1 )) ) {
+		trap->FS_Close( f );
 		return;
 	}
 
@@ -917,7 +920,6 @@ void AM_SaveTelemarks( void ) {
 	Com_sprintf( loadPath, sizeof(loadPath), "telemarks" PATH_SEP "%s.json", level.rawmapname );
 	trap->FS_Open( loadPath, &f, FS_WRITE );
 	Com_Printf( "Saving telemarks (%s)\n", loadPath );
-
 	AM_WriteTelemarks( f );
 }
 

diff --git a/game/g_geoip.cpp b/game/g_geoip.cpp
@@ -181,6 +181,7 @@ namespace GeoIP {
 		// alloc memory for buffer
 		char *buf = (char *)malloc( len + 1 );
 		if ( !buf ) {
+			trap->FS_Close( f );
 			return false;
 		}
 

diff --git a/game/g_main.cpp b/game/g_main.cpp
@@ -424,7 +424,7 @@ static void G_SpawnHoleFixes( void ) {
 	Com_sprintf( filename, sizeof(filename), "%s_holes.cfg", mapname );
 
 	len = trap->FS_Open( filename, &f, FS_READ );
-	if ( len != -1 ) {
+	if ( f && len != -1 ) {
 		// read mins, maxs out of file
 		char *text = (char *)calloc( len + 1, 1 );
 		const char *cursor = text;

diff --git a/game/g_saga.cpp b/game/g_saga.cpp
@@ -122,7 +122,11 @@ void InitSiegeMode( void ) {
 
 	len = trap->FS_Open( levelname, &f, FS_READ );
 
-	if ( !f || len >= MAX_SIEGE_INFO_SIZE ) {
+	if ( !f ) {
+		goto failure;
+	}
+	if ( len >= MAX_SIEGE_INFO_SIZE ) {
+		trap->FS_Close( f );
 		goto failure;
 	}
 

diff --git a/game/g_session.cpp b/game/g_session.cpp
@@ -68,6 +68,7 @@ void G_ReadClientSessionData( gclient_t *client ) {
 
 	buffer = (char *)malloc( len + 1 );
 	if ( !buffer ) {
+		trap->FS_Close( f );
 		return;
 	}
 

diff --git a/ui/ui_force.cpp b/ui/ui_force.cpp
@@ -1019,6 +1019,7 @@ void UI_ForceConfigHandle( int oldindex, int newindex ) {
 	}
 
 	if ( len >= 8192 ) {
+		trap->FS_Close( f );
 		return;
 	}
 

diff --git a/ui/ui_main.cpp b/ui/ui_main.cpp
@@ -276,10 +276,12 @@ int UI_ParseAnimationFile( const char *filename, animation_t *animset, qboolean
 	// load the file
 	if ( !UIPAFtextLoaded || !isHumanoid ) { //rww - We are always using the same animation config now. So only load it once.
 		len = trap->FS_Open( filename, &f, FS_READ );
-		if ( (len <= 0) || (len >= (signed)sizeof(UIPAFtext)-1) ) {
-			if ( len > 0 ) {
-				Com_Error( ERR_DROP, "%s exceeds the allowed ui-side animation buffer!", filename );
-			}
+		if ( !f || len <= 0 ) {
+			return -1;
+		}
+		if ( len >= (signed)sizeof(UIPAFtext)-1 ) {
+			trap->FS_Close( f );
+			Com_Error( ERR_DROP, "%s exceeds the allowed ui-side animation buffer!", filename );
 			return -1;
 		}
 
@@ -5836,7 +5838,11 @@ void UI_SetSiegeTeams( void ) {
 
 	len = trap->FS_Open( levelname, &f, FS_READ );
 
-	if ( !f || len >= MAX_SIEGE_INFO_SIZE ) {
+	if ( !f ) {
+		return;
+	}
+	if ( len >= MAX_SIEGE_INFO_SIZE ) {
+		trap->FS_Close( f );
 		return;
 	}
 
@@ -7897,6 +7903,7 @@ static void UI_BuildPlayerModel_List( qboolean inGameLoad ) {
 
 			char *buffer = (char *)malloc( fileLen + 1 );
 			if ( !buffer ) {
+				trap->FS_Close( f );
 				Com_Error( ERR_FATAL, "Could not allocate buffer to read %s", fPath );
 			}
 
@@ -8749,6 +8756,7 @@ void JP_ParseFavServers( void )
 
 	if ( (buf = (char*)malloc(len+1)) == 0 )
 	{
+		trap->FS_Close( f );
 		Com_Printf( S_COLOR_RED"failed! "S_COLOR_WHITE"(Failed to allocate buffer)\n" );
 		return;
 	}