[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)
cfinck at svn.reactos.org
cfinck at svn.reactos.org
Fri Feb 13 18:39:59 CET 2009
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=39584&r1=39583&r2=39584&view=diff
==============================================================================
--- 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?rev=39584&r1=39583&r2=39584&view=diff
==============================================================================
--- 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?rev=39584&r1=39583&r2=39584&view=diff
==============================================================================
--- 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;
}
More information about the Ros-diffs
mailing list