[ros-diffs] [cfinck] 39584: - Add some checks to prevent crashes in unexpected situations and add useful error messages for them. This should make debugging something like r39578 easier :-) - Prevent some memory leaks in case of failure (well, some memory wasn't even freed in case of success :-P)

Author: cfinck
Date: Fri Feb 13 11:39:58 2009
New Revision: 39584

URL: http://svn.reactos.org/svn/reactos?rev=39584&view=rev
Log:
- Add some checks to prevent crashes in unexpected situations and add useful error
messages for them.
  This should make debugging something like r39578 easier :-)
- Prevent some memory leaks in case of failure (well, some memory wasn't even freed in
case of success :-P)

Modified:
    trunk/rostests/rosautotest/main.c
    trunk/rostests/rosautotest/webservice.c
    trunk/rostests/rosautotest/winetests.c

Modified: trunk/rostests/rosautotest/main.c
URL:
http://svn.reactos.org/svn/reactos/trunk/rostests/rosautotest/main.c?rev=39…
==============================================================================
--- trunk/rostests/rosautotest/main.c [iso-8859-1] (original)
+++ trunk/rostests/rosautotest/main.c [iso-8859-1] Fri Feb 13 11:39:58 2009
@@ -66,15 +66,19 @@
     const CHAR PasswordProp[] = "&password=";
     const CHAR UserNameProp[] = "&username=";
 
+    BOOL ReturnValue = FALSE;
     DWORD DataLength;
     DWORD Length;
-    PCHAR Password;
-    PCHAR UserName;
+    PCHAR Password = NULL;
+    PCHAR UserName = NULL;
     WCHAR ConfigFile[MAX_PATH];
 
     /* We only need this if the results are going to be submitted */
     if(!AppOptions.Submit)
-        return TRUE;
+    {
+        ReturnValue = TRUE;
+        goto Cleanup;
+    }
 
     /* Build the path to the configuration file from the application's path */
     GetModuleFileNameW(NULL, ConfigFile, MAX_PATH);
@@ -85,7 +89,7 @@
     if(GetFileAttributesW(ConfigFile) == INVALID_FILE_ATTRIBUTES)
     {
         StringOut("Missing \"rosautotest.ini\" configuration
file!\n");
-        return FALSE;
+        goto Cleanup;
     }
 
     /* Get the required length of the authentication request string */
@@ -95,7 +99,7 @@
     if(!Length)
     {
         StringOut("UserName is missing in the configuration file\n");
-        return FALSE;
+        goto Cleanup;
     }
 
     /* Some characters might need to be escaped and an escaped character takes 3 bytes
*/
@@ -107,7 +111,7 @@
     if(!Length)
     {
         StringOut("Password is missing in the configuration file\n");
-        return FALSE;
+        goto Cleanup;
     }
 
     DataLength += 3 * Length;
@@ -121,7 +125,16 @@
     strcat(AuthenticationRequestString, PasswordProp);
     EscapeString(&AuthenticationRequestString[strlen(AuthenticationRequestString)],
Password);
 
-    return TRUE;
+    ReturnValue = TRUE;
+
+Cleanup:
+    if(UserName)
+        HeapFree(hProcessHeap, 0, UserName);
+
+    if(Password)
+        HeapFree(hProcessHeap, 0, Password);
+
+    return ReturnValue;
 }
 
 /**
@@ -242,7 +255,7 @@
 int
 wmain(int argc, wchar_t* argv[])
 {
-    int Result = 0;
+    int ReturnValue = 0;
     UINT i;
 
     hProcessHeap = GetProcessHeap();
@@ -263,12 +276,12 @@
                     break;
 
                 default:
-                    Result = 1;
+                    ReturnValue = 1;
                     /* Fall through */
 
                 case '?':
                     IntPrintUsage();
-                    goto End;
+                    goto Cleanup;
             }
         }
         else
@@ -292,24 +305,23 @@
             }
             else
             {
-                Result = 1;
+                ReturnValue = 1;
                 IntPrintUsage();
-                goto End;
+                goto Cleanup;
             }
         }
     }
 
     if(!IntGetConfigurationValues() || !IntGetBuildAndPlatform() || !RunWineTests())
     {
-        Result = 1;
-        goto End;
+        ReturnValue = 1;
+        goto Cleanup;
     }
 
     /* For sysreg */
     OutputDebugStringA("SYSREG_CHECKPOINT:THIRDBOOT_COMPLETE\n");
 
-End:
-    /* Cleanup */
+Cleanup:
     if(AppOptions.Module)
         HeapFree(hProcessHeap, 0, AppOptions.Module);
 
@@ -324,7 +336,7 @@
 
     /* Shut down the system if requested */
     if(AppOptions.Shutdown && !ShutdownSystem())
-        Result = 1;
-
-    return Result;
-}
+        ReturnValue = 1;
+
+    return ReturnValue;
+}

Modified: trunk/rostests/rosautotest/webservice.c
URL:
http://svn.reactos.org/svn/reactos/trunk/rostests/rosautotest/webservice.c?…
==============================================================================
--- trunk/rostests/rosautotest/webservice.c [iso-8859-1] (original)
+++ trunk/rostests/rosautotest/webservice.c [iso-8859-1] Fri Feb 13 11:39:58 2009
@@ -33,9 +33,10 @@
 {
     const WCHAR Headers[] = L"Content-Type:
application/x-www-form-urlencoded";
 
-    HINTERNET hHTTP;
-    HINTERNET hHTTPRequest;
-    HINTERNET hInet;
+    BOOL ReturnValue = FALSE;
+    HINTERNET hHTTP = NULL;
+    HINTERNET hHTTPRequest = NULL;
+    HINTERNET hInet = NULL;
 
     /* Establish an internet connection to the "testman" server */
     hInet = InternetOpenW(L"rosautotest", INTERNET_OPEN_TYPE_PRECONFIG, NULL,
NULL, 0);
@@ -43,7 +44,7 @@
     if(!hInet)
     {
         StringOut("InternetOpenW failed\n");
-        return FALSE;
+        goto Cleanup;
     }
 
     hHTTP = InternetConnectW(hInet, SERVER_HOSTNAME, INTERNET_DEFAULT_HTTP_PORT, NULL,
NULL, INTERNET_SERVICE_HTTP, 0, 0);
@@ -51,7 +52,7 @@
     if(!hHTTP)
     {
         StringOut("InternetConnectW failed\n");
-        return FALSE;
+        goto Cleanup;
     }
 
     /* Post our test results to the web service */
@@ -60,22 +61,23 @@
     if(!hHTTPRequest)
     {
         StringOut("HttpOpenRequestW failed\n");
-        return FALSE;
+        goto Cleanup;
     }
 
     if(!HttpSendRequestW(hHTTPRequest, Headers, wcslen(Headers), *Data, *DataLength))
     {
         StringOut("HttpSendRequestW failed\n");
-        return FALSE;
+        goto Cleanup;
     }
 
     HeapFree(hProcessHeap, 0, *Data);
+    *Data = NULL;
 
     /* Get the response */
     if(!InternetQueryDataAvailable(hHTTPRequest, DataLength, 0, 0))
     {
         StringOut("InternetQueryDataAvailable failed\n");
-        return FALSE;
+        goto Cleanup;
     }
 
     *Data = HeapAlloc(hProcessHeap, 0, *DataLength + 1);
@@ -83,16 +85,23 @@
     if(!InternetReadFile(hHTTPRequest, *Data, *DataLength, DataLength))
     {
         StringOut("InternetReadFile failed\n");
-        return FALSE;
+        goto Cleanup;
     }
 
     (*Data)[*DataLength] = 0;
-
-    InternetCloseHandle(hHTTPRequest);
-    InternetCloseHandle(hHTTP);
-    InternetCloseHandle(hInet);
-
-    return TRUE;
+    ReturnValue = TRUE;
+
+Cleanup:
+    if(hHTTPRequest)
+        InternetCloseHandle(hHTTPRequest);
+
+    if(hHTTP)
+        InternetCloseHandle(hHTTP);
+
+    if(hInet)
+        InternetCloseHandle(hInet);
+
+    return ReturnValue;
 }
 
 /**
@@ -127,6 +136,7 @@
  *
  * @return
  * Returns the Test ID as a CHAR array if successful or NULL otherwise.
+ * The caller needs to HeapFree the returned pointer in case of success.
  */
 PCHAR
 GetTestID(TESTTYPES TestType)
@@ -135,6 +145,7 @@
 
     DWORD DataLength;
     PCHAR Data;
+    PCHAR ReturnValue = NULL;
 
     /* Build the full request string */
     DataLength  = sizeof(ActionProp) - 1 + sizeof(GetTestIDAction) - 1;
@@ -163,7 +174,7 @@
     }
 
     if(!IntDoRequest(&Data, &DataLength))
-        return NULL;
+        goto Cleanup;
 
     /* Verify that this is really a number */
     if(!IsNumber(Data))
@@ -171,11 +182,16 @@
         StringOut("Expected Test ID, but received:\n");
         StringOut(Data);
         StringOut("\n");
+        goto Cleanup;
+    }
+
+    ReturnValue = Data;
+
+Cleanup:
+    if(Data && ReturnValue != Data)
         HeapFree(hProcessHeap, 0, Data);
-        return NULL;
-    }
-
-    return Data;
+
+    return ReturnValue;
 }
 
 /**
@@ -190,6 +206,7 @@
  *
  * @return
  * Returns the Suite ID as a CHAR array if successful or NULL otherwise.
+ * The caller needs to HeapFree the returned pointer in case of success.
  */
 PCHAR
 GetSuiteID(TESTTYPES TestType, const PVOID TestData)
@@ -200,6 +217,7 @@
 
     DWORD DataLength;
     PCHAR Data;
+    PCHAR ReturnValue = NULL;
     PWINE_GETSUITEID_DATA WineData;
 
     DataLength  = sizeof(ActionProp) - 1 + sizeof(GetSuiteIDAction) - 1;
@@ -242,7 +260,7 @@
     }
 
     if(!IntDoRequest(&Data, &DataLength))
-        return NULL;
+        goto Cleanup;
 
     /* Verify that this is really a number */
     if(!IsNumber(Data))
@@ -250,11 +268,16 @@
         StringOut("Expected Suite ID, but received:\n");
         StringOut(Data);
         StringOut("\n");
+        goto Cleanup;
+    }
+
+    ReturnValue = Data;
+
+Cleanup:
+    if(Data && ReturnValue != Data)
         HeapFree(hProcessHeap, 0, Data);
-        return NULL;
-    }
-
-    return Data;
+
+    return ReturnValue;
 }
 
 /**
@@ -277,6 +300,7 @@
     const CHAR SuiteIDProp[] = "&suiteid=";
     const CHAR LogProp[] = "&log=";
 
+    BOOL ReturnValue = FALSE;
     DWORD DataLength;
     PCHAR Data;
     PCHAR pData;
@@ -342,7 +366,7 @@
 
     /* Send all the stuff */
     if(!IntDoRequest(&Data, &DataLength))
-        return FALSE;
+        goto Cleanup;
 
     /* Output the response */
     StringOut("The server responded:\n");
@@ -350,9 +374,13 @@
     StringOut("\n");
 
     if(!strcmp(Data, "OK"))
-        return TRUE;
-
-    return FALSE;
+        ReturnValue = TRUE;
+
+Cleanup:
+    if(Data)
+        HeapFree(hProcessHeap, 0, Data);
+
+    return ReturnValue;
 }
 
 /**
@@ -373,6 +401,7 @@
 {
     const CHAR FinishAction[] = "finish";
 
+    BOOL ReturnValue = FALSE;
     DWORD DataLength;
     PCHAR Data;
     PGENERAL_FINISH_DATA GeneralData;
@@ -410,10 +439,14 @@
     }
 
     if(!IntDoRequest(&Data, &DataLength))
-        return FALSE;
+        goto Cleanup;
 
     if(!strcmp(Data, "OK"))
-        return TRUE;
-
-    return FALSE;
-}
+        ReturnValue = TRUE;
+
+Cleanup:
+    if(Data)
+        HeapFree(hProcessHeap, 0, Data);
+
+    return ReturnValue;
+}

Modified: trunk/rostests/rosautotest/winetests.c
URL:
http://svn.reactos.org/svn/reactos/trunk/rostests/rosautotest/winetests.c?r…
==============================================================================
--- trunk/rostests/rosautotest/winetests.c [iso-8859-1] (original)
+++ trunk/rostests/rosautotest/winetests.c [iso-8859-1] Fri Feb 13 11:39:58 2009
@@ -33,13 +33,14 @@
 IntRunTest(PWSTR CommandLine, HANDLE hReadPipe, LPSTARTUPINFOW StartupInfo,
PWINE_GETSUITEID_DATA GetSuiteIDData, PWINE_SUBMIT_DATA SubmitData)
 {
     BOOL BreakLoop = FALSE;
+    BOOL ReturnValue = FALSE;
     DWORD BytesAvailable;
     DWORD LogAvailable = 0;
     DWORD LogLength = 0;
     DWORD LogPosition = 0;
     DWORD Temp;
-    PCHAR Buffer;
-    PROCESS_INFORMATION ProcessInfo;
+    PCHAR Buffer = NULL;
+    PROCESS_INFORMATION ProcessInfo = {0};
 
     if(AppOptions.Submit)
     {
@@ -55,12 +56,13 @@
     sprintf(Buffer, "Running Wine Test, Module: %s, Test: %s\n",
GetSuiteIDData->Module, GetSuiteIDData->Test);
     StringOut(Buffer);
     HeapFree(hProcessHeap, 0, Buffer);
+    Buffer = NULL;
 
     /* Execute the test */
     if(!CreateProcessW(NULL, CommandLine, NULL, NULL, TRUE, NORMAL_PRIORITY_CLASS, NULL,
NULL, StartupInfo, &ProcessInfo))
     {
         StringOut("CreateProcessW for running the test failed\n");
-        return FALSE;
+        goto Cleanup;
     }
 
     /* Receive all the data from the pipe */
@@ -78,7 +80,7 @@
         if(!PeekNamedPipe(hReadPipe, NULL, 0, NULL, &BytesAvailable, NULL))
         {
             StringOut("PeekNamedPipe failed for the test run\n");
-            return FALSE;
+            goto Cleanup;
         }
 
         if(BytesAvailable)
@@ -89,7 +91,7 @@
             if(!ReadFile(hReadPipe, Buffer, BytesAvailable, &Temp, NULL))
             {
                 StringOut("ReadFile failed for the test run\n");
-                return FALSE;
+                goto Cleanup;
             }
 
             /* Output all test output through StringOut, even while the test is still
running */
@@ -114,13 +116,10 @@
             }
 
             HeapFree(hProcessHeap, 0, Buffer);
+            Buffer = NULL;
         }
     }
     while(!BreakLoop);
-
-    /* Close the process handles */
-    CloseHandle(ProcessInfo.hProcess);
-    CloseHandle(ProcessInfo.hThread);
 
     if(AppOptions.Submit)
     {
@@ -137,29 +136,47 @@
                 SubmitData->General.TestID = GetTestID(WineTest);
 
                 if(!SubmitData->General.TestID)
-                    return FALSE;
+                    goto Cleanup;
             }
 
             /* Get a Suite ID for this combination */
             SubmitData->General.SuiteID = GetSuiteID(WineTest, GetSuiteIDData);
 
             if(!SubmitData->General.SuiteID)
-                return FALSE;
+                goto Cleanup;
 
             /* Submit the stuff */
             Submit(WineTest, SubmitData);
-
-            /* Cleanup */
-            HeapFree(hProcessHeap, 0, SubmitData->General.SuiteID);
         }
-
-        /* Cleanup */
+    }
+
+    StringOut("\n\n");
+
+    ReturnValue = TRUE;
+
+Cleanup:
+    if(Buffer)
+        HeapFree(hProcessHeap, 0, Buffer);
+
+    if(ProcessInfo.hProcess)
+        HeapFree(hProcessHeap, 0, ProcessInfo.hProcess);
+
+    if(ProcessInfo.hThread)
+        HeapFree(hProcessHeap, 0, ProcessInfo.hThread);
+
+    if(SubmitData->General.SuiteID)
+    {
+        HeapFree(hProcessHeap, 0, SubmitData->General.SuiteID);
+        SubmitData->General.SuiteID = NULL;
+    }
+
+    if(SubmitData->Log)
+    {
         HeapFree(hProcessHeap, 0, SubmitData->Log);
-    }
-
-    StringOut("\n\n");
-
-    return TRUE;
+        SubmitData->Log = NULL;
+    }
+
+    return ReturnValue;
 }
 
 /**
@@ -186,15 +203,17 @@
 static BOOL
 IntRunModuleTests(PWSTR File, PWSTR FilePath, HANDLE hReadPipe, LPSTARTUPINFOW
StartupInfo, PWINE_SUBMIT_DATA SubmitData)
 {
+    BOOL ReturnValue = FALSE;
     DWORD BytesAvailable;
     DWORD Length;
     DWORD Temp;
-    PCHAR Buffer;
+    PCHAR Buffer = NULL;
     PCHAR pStart;
     PCHAR pEnd;
-    PROCESS_INFORMATION ProcessInfo;
+    PROCESS_INFORMATION ProcessInfo = {0};
+    PWSTR pUnderscore;
     size_t FilePosition;
-    WINE_GETSUITEID_DATA GetSuiteIDData;
+    WINE_GETSUITEID_DATA GetSuiteIDData = {0};
 
     /* Build the full command line */
     FilePosition = wcslen(FilePath);
@@ -202,8 +221,25 @@
     FilePath[FilePosition] = 0;
     wcscat(FilePath, L"--list");
 
+    /* Find the underscore in the file name */
+    pUnderscore = wcschr(File, L'_');
+
+    if(!pUnderscore)
+    {
+        StringOut("Invalid test file name: ");
+
+        Length = wcslen(File);
+        Buffer = HeapAlloc(hProcessHeap, 0, Length + 1);
+        WideCharToMultiByte(CP_ACP, 0, File, Length + 1, Buffer, Length + 1, NULL,
NULL);
+
+        StringOut(Buffer);
+        StringOut("\n");
+
+        goto Cleanup;
+    }
+
     /* Store the tested module name */
-    Length = wcschr(File, L'_') - File;
+    Length = pUnderscore - File;
     GetSuiteIDData.Module = HeapAlloc(hProcessHeap, 0, Length + 1);
     WideCharToMultiByte(CP_ACP, 0, File, Length, GetSuiteIDData.Module, Length, NULL,
NULL);
     GetSuiteIDData.Module[Length] = 0;
@@ -212,33 +248,45 @@
     if(!CreateProcessW(NULL, FilePath, NULL, NULL, TRUE, NORMAL_PRIORITY_CLASS, NULL,
NULL, StartupInfo, &ProcessInfo))
     {
         StringOut("CreateProcessW for getting the available tests failed\n");
-        return FALSE;
+        goto Cleanup;
     }
 
     /* Wait till this process ended */
     if(WaitForSingleObject(ProcessInfo.hProcess, INFINITE) == WAIT_FAILED)
     {
         StringOut("WaitForSingleObject failed for the test list\n");
-        return FALSE;
-    }
-
-    /* Close the process handles */
-    CloseHandle(ProcessInfo.hProcess);
-    CloseHandle(ProcessInfo.hThread);
+        goto Cleanup;
+    }
 
     /* Read the output data into a buffer */
     if(!PeekNamedPipe(hReadPipe, NULL, 0, NULL, &BytesAvailable, NULL))
     {
         StringOut("PeekNamedPipe failed for the test list\n");
-        return FALSE;
-    }
-
+        goto Cleanup;
+    }
+
+    /* Check if we got any */
+    if(!BytesAvailable)
+    {
+        StringOut("The --list command did not return any data for ");
+
+        Length = wcslen(File);
+        Buffer = HeapAlloc(hProcessHeap, 0, Length + 1);
+        WideCharToMultiByte(CP_ACP, 0, File, Length + 1, Buffer, Length + 1, NULL,
NULL);
+
+        StringOut(Buffer);
+        StringOut("\n");
+
+        goto Cleanup;
+    }
+
+    /* Read the data */
     Buffer = HeapAlloc(hProcessHeap, 0, BytesAvailable);
 
     if(!ReadFile(hReadPipe, Buffer, BytesAvailable, &Temp, NULL))
     {
         StringOut("ReadFile failed\n");
-        return FALSE;
+        goto Cleanup;
     }
 
     /* Jump to the first available test */
@@ -267,21 +315,36 @@
             FilePath[FilePosition + Length] = 0;
 
             if(!IntRunTest(FilePath, hReadPipe, StartupInfo, &GetSuiteIDData,
SubmitData))
-                return FALSE;
+                goto Cleanup;
         }
 
         /* Cleanup */
         HeapFree(hProcessHeap, 0, GetSuiteIDData.Test);
+        GetSuiteIDData.Test = NULL;
 
         /* Move to the next test */
         pStart = pEnd + 6;
     }
 
-    /* Cleanup */
-    HeapFree(hProcessHeap, 0, GetSuiteIDData.Module);
-    HeapFree(hProcessHeap, 0, Buffer);
-
-    return TRUE;
+    ReturnValue = TRUE;
+
+Cleanup:
+    if(GetSuiteIDData.Module)
+        HeapFree(hProcessHeap, 0, GetSuiteIDData.Module);
+
+    if(GetSuiteIDData.Test)
+        HeapFree(hProcessHeap, 0, GetSuiteIDData.Test);
+
+    if(Buffer)
+        HeapFree(hProcessHeap, 0, Buffer);
+
+    if(ProcessInfo.hProcess)
+        CloseHandle(ProcessInfo.hProcess);
+
+    if(ProcessInfo.hThread)
+        CloseHandle(ProcessInfo.hThread);
+
+    return ReturnValue;
 }
 
 /**
@@ -293,10 +356,11 @@
 BOOL
 RunWineTests()
 {
+    BOOL ReturnValue = FALSE;
     GENERAL_FINISH_DATA FinishData;
-    HANDLE hFind;
-    HANDLE hReadPipe;
-    HANDLE hWritePipe;
+    HANDLE hFind = NULL;
+    HANDLE hReadPipe = NULL;
+    HANDLE hWritePipe = NULL;
     SECURITY_ATTRIBUTES SecurityAttributes;
     STARTUPINFOW StartupInfo = {0};
     size_t PathPosition;
@@ -312,7 +376,7 @@
     if(!CreatePipe(&hReadPipe, &hWritePipe, &SecurityAttributes, 0))
     {
         StringOut("CreatePipe failed\n");
-        return FALSE;
+        goto Cleanup;
     }
 
     StartupInfo.cb = sizeof(StartupInfo);
@@ -323,7 +387,7 @@
     if(GetWindowsDirectoryW(FilePath, MAX_PATH) > MAX_PATH - 60)
     {
         StringOut("Windows directory path is too long\n");
-        return FALSE;
+        goto Cleanup;
     }
 
     wcscat(FilePath, L"\\bin\\");
@@ -346,7 +410,7 @@
     if(hFind == INVALID_HANDLE_VALUE)
     {
         StringOut("FindFirstFileW failed\n");
-        return FALSE;
+        goto Cleanup;
     }
 
     /* Run the tests */
@@ -357,27 +421,33 @@
 
         /* Run it */
         if(!IntRunModuleTests(fd.cFileName, FilePath, hReadPipe, &StartupInfo,
&SubmitData))
-            return FALSE;
+            goto Cleanup;
     }
     while(FindNextFileW(hFind, &fd));
 
-    /* Cleanup */
-    FindClose(hFind);
-
-    if(AppOptions.Submit && SubmitData.General.TestID)
-    {
-        /* We're done with the tests, so close this test run */
+    /* Close this test run if necessary */
+    if(SubmitData.General.TestID)
+    {
         FinishData.TestID = SubmitData.General.TestID;
 
         if(!Finish(WineTest, &FinishData))
-            return FALSE;
-
-        /* Cleanup */
-        HeapFree(hProcessHeap, 0, FinishData.TestID);
-    }
-
-    CloseHandle(hReadPipe);
-    CloseHandle(hWritePipe);
-
-    return TRUE;
+            goto Cleanup;
+    }
+
+    ReturnValue = TRUE;
+
+Cleanup:
+    if(SubmitData.General.TestID)
+        HeapFree(hProcessHeap, 0, SubmitData.General.TestID);
+
+    if(hFind && hFind != INVALID_HANDLE_VALUE)
+        FindClose(hFind);
+
+    if(hReadPipe)
+        CloseHandle(hReadPipe);
+
+    if(hWritePipe)
+        CloseHandle(hWritePipe);
+
+    return ReturnValue;
 }