STORM-1552: detect, ignore, and delete invalid feature and gpu table files

indra/newview/gpu_table.txt

+//GPU_TABLE - that token on line 1 tags this as a gpu table file
 //
 // Categorizes graphics chips into various classes by name
 //

indra/newview/llfeaturemanager.cpp

@@ -261,7 +261,7 @@ BOOL LLFeatureManager::maskFeatures(const std::string& name)
 	return maskList(*maskp);
 }
 
-BOOL LLFeatureManager::loadFeatureTables()
+bool LLFeatureManager::loadFeatureTables()
 {
 	// *TODO - if I or anyone else adds something else to the skipped list
 	// make this data driven.  Put it in the feature table and parse it
@@ -302,28 +302,36 @@ BOOL LLFeatureManager::loadFeatureTables()
 
 	// use HTTP table if it exists
 	std::string path;
+	bool parse_ok = false;
 	if (gDirUtilp->fileExists(http_path))
 	{
-		path = http_path;
+		parse_ok = parseFeatureTable(http_path);
+		if (!parse_ok)
+		{
+			// the HTTP table failed to parse, so delete it
+			LLFile::remove(http_path);
+			LL_WARNS("RenderInit") << "Removed invalid feature table '" << http_path << "'" << LL_ENDL;
+		}
 	}
-	else
+
+	if (!parse_ok)
 	{
-		path = app_path;
+		parse_ok = parseFeatureTable(app_path);
 	}
 
-	
-	return parseFeatureTable(path);
+	return parse_ok;
 }
 
 
-BOOL LLFeatureManager::parseFeatureTable(std::string filename)
+bool LLFeatureManager::parseFeatureTable(std::string filename)
 {
-	llinfos << "Looking for feature table in " << filename << llendl;
+	LL_INFOS("RenderInit") << "Attempting to parse feature table from " << filename << LL_ENDL;
 
 	llifstream file;
 	std::string name;
 	U32		version;
 	
+	cleanupFeatureTables(); // in case an earlier attempt left partial results
 	file.open(filename); 	 /*Flawfinder: ignore*/
 
 	if (!file)
@@ -338,13 +346,14 @@ BOOL LLFeatureManager::parseFeatureTable(std::string filename)
 	if (name != "version")
 	{
 		LL_WARNS("RenderInit") << filename << " does not appear to be a valid feature table!" << LL_ENDL;
-		return FALSE;
+		return false;
 	}
 
 	mTableVersion = version;
-
+	
 	LLFeatureList *flp = NULL;
-	while (file >> name)
+	bool parse_ok = true;
+	while (file >> name && parse_ok)
 	{
 		char buffer[MAX_STRING];		 /*Flawfinder: ignore*/
 		
@@ -357,39 +366,58 @@ BOOL LLFeatureManager::parseFeatureTable(std::string filename)
 
 		if (name == "list")
 		{
+			LL_DEBUGS("RenderInit") << "Before new list" << std::endl;
 			if (flp)
 			{
-				//flp->dump();
+				flp->dump();
 			}
+			else
+			{
+				LL_CONT << "No current list";
+			}
+			LL_CONT << LL_ENDL;
+			
 			// It's a new mask, create it.
 			file >> name;
-			if (mMaskList.count(name))
+			if (!mMaskList.count(name))
 			{
-				LL_ERRS("RenderInit") << "Overriding mask " << name << ", this is invalid!" << LL_ENDL;
+				flp = new LLFeatureList(name);
+				mMaskList[name] = flp;
+			}
+			else
+			{
+				LL_WARNS("RenderInit") << "Overriding mask " << name << ", this is invalid!" << LL_ENDL;
+				parse_ok = false;
 			}
-
-			flp = new LLFeatureList(name);
-			mMaskList[name] = flp;
 		}
 		else
 		{
 			if (!flp)
 			{
-				LL_ERRS("RenderInit") << "Specified parameter before <list> keyword!" << LL_ENDL;
-				return FALSE;
+				S32 available;
+				F32 recommended;
+				file >> available >> recommended;
+				flp->addFeature(name, available, recommended);
+			}
+			else
+			{
+				LL_WARNS("RenderInit") << "Specified parameter before <list> keyword!" << LL_ENDL;
+				parse_ok = false;
 			}
-			S32 available;
-			F32 recommended;
-			file >> available >> recommended;
-			flp->addFeature(name, available, recommended);
 		}
 	}
 	file.close();
 
-	return TRUE;
+	if (!parse_ok)
+	{
+		LL_WARNS("RenderInit") << "Discarding feature table data from " << filename << LL_ENDL;
+		cleanupFeatureTables();
+	}
+	
+	return parse_ok;
 }
 
-void LLFeatureManager::loadGPUClass()
+bool LLFeatureManager::loadGPUClass()
 {
 	// defaults
 	mGPUClass = GPU_CLASS_UNKNOWN;
@@ -407,29 +435,49 @@ void LLFeatureManager::loadGPUClass()
 
 	// use HTTP table if it exists
 	std::string path;
+	bool parse_ok = false;
 	if (gDirUtilp->fileExists(http_path))
 	{
-		path = http_path;
+		parse_ok = parseGPUTable(http_path);
+		if (!parse_ok)
+		{
+			// the HTTP table failed to parse, so delete it
+			LLFile::remove(http_path);
+			LL_WARNS("RenderInit") << "Removed invalid gpu table '" << http_path << "'" << LL_ENDL;
+		}
 	}
-	else
+
+	if (!parse_ok)
 	{
-		path = app_path;
+		parse_ok = parseGPUTable(app_path);
 	}
 
-	parseGPUTable(path);
+	return parse_ok; // indicates that the file parsed correctly, not that the gpu was recognized
 }
 
 	
-void LLFeatureManager::parseGPUTable(std::string filename)
+bool LLFeatureManager::parseGPUTable(std::string filename)
 {
 	llifstream file;
-		
+	
+	LL_INFOS("RenderInit") << "Attempting to parse GPU table from " << filename << LL_ENDL;
 	file.open(filename);
 
-	if (!file)
+	if (file)
+	{
+		const char recognizer[] = "//GPU_TABLE";
+		char first_line[MAX_STRING];
+		file.getline(first_line, MAX_STRING);
+		if (0 != strncmp(first_line, recognizer, strlen(recognizer)))
+		{
+			LL_WARNS("RenderInit") << "Invalid GPU table: " << filename << "!" << LL_ENDL;
+			return false;
+		}
+	}
+	else
 	{
 		LL_WARNS("RenderInit") << "Unable to open GPU table: " << filename << "!" << LL_ENDL;
-		return;
+		return false;
 	}
 
 	std::string rawRenderer = gGLManager.getRawGLString();
@@ -556,6 +604,7 @@ void LLFeatureManager::parseGPUTable(std::string filename)
 #if LL_DARWIN // never go over "Mid" settings by default on OS X
 	mGPUClass = llmin(mGPUClass, GPU_CLASS_2);
 #endif
+	return true;
 }
 
 // responder saves table into file

indra/newview/llfeaturemanager.h

@@ -75,7 +75,7 @@ class LLFeatureList
 	void setFeatureAvailable(const std::string& name, const BOOL available);
 	void setRecommendedLevel(const std::string& name, const F32 level);
 
-	BOOL loadFeatureList(LLFILE *fp);
+	bool loadFeatureList(LLFILE *fp);
 
 	BOOL maskList(LLFeatureList &mask);
 
@@ -114,7 +114,7 @@ class LLFeatureManager : public LLFeatureList, public LLSingleton<LLFeatureManag
 
 	void maskCurrentList(const std::string& name); // Mask the current feature list with the named list
 
-	BOOL loadFeatureTables();
+	bool loadFeatureTables();
 
 	EGPUClass getGPUClass() 			{ return mGPUClass; }
 	std::string& getGPUString() 		{ return mGPUString; }
@@ -157,9 +157,14 @@ class LLFeatureManager : public LLFeatureList, public LLSingleton<LLFeatureManag
 	void fetchHTTPTables();
 	
 protected:
-	void loadGPUClass();
-	BOOL parseFeatureTable(std::string filename);
-	void parseGPUTable(std::string filename);
+	bool loadGPUClass();
+
+	bool parseFeatureTable(std::string filename);
+	///< @returns TRUE is file parsed correctly, FALSE if not
+
+	bool parseGPUTable(std::string filename);
+	///< @returns true if file parsed correctly, false if not - does not reflect whether or not the gpu was recognized
+
 	void initBaseMask();
 
 

scripts/gpu_table_tester

@@ -73,6 +73,10 @@ die "Must specify a --gpu-table <gpu_table.txt> value"
 open(GPUS, "<$GpuTable")
     || die "Failed to open gpu table '$GpuTable':\n\t$!\n";
 
+my $FirstLine = <GPUS>;
+die "First line of gpu table does not begin with '//GPU_TABLE'"
+    unless $FirstLine =~ m|^//GPU_TABLE|;
+
 # Parse the GPU table into these tables, indexed by the name
 my %NameLine;       # name -> line number on which a given name was found (catches duplicate names)
 my %RecognizerLine; # name -> line number on which a given name was found (catches duplicate names)