[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=395... ============================================================================== --- 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?r... ============================================================================== --- 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?re... ============================================================================== --- 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; }
-
cfinck＠svn.reactos.org