From 56399280be8f42f8c4813387635a577b74e26f26 Mon Sep 17 00:00:00 2001 From: hkosaka Date: Thu, 11 Apr 2013 11:33:52 +0000 Subject: [PATCH] 2013-04-11 Hirofumi Kosaka * win32/pandora_windows_service.cc: Fixed possible resource leaks; - putenv() could run out the environment variable area, if using both file_collection and broker_agent. - possible handle leak at launching tentacle_client. * Cleaned source code style. Merged from branch 4.x. git-svn-id: https://svn.code.sf.net/p/pandora/code/trunk@7962 c3f86ba8-e40f-0410-aaad-9ba5e7f4b01f --- pandora_agents/ChangeLog | 10 + pandora_agents/win32/ChangeLog | 10 + .../win32/pandora_windows_service.cc | 177 ++++++++++-------- 3 files changed, 119 insertions(+), 78 deletions(-) diff --git a/pandora_agents/ChangeLog b/pandora_agents/ChangeLog index a75547811a..c5918d656a 100644 --- a/pandora_agents/ChangeLog +++ b/pandora_agents/ChangeLog @@ -1,3 +1,13 @@ +2013-04-11 Hirofumi Kosaka + + * win32/pandora_windows_service.cc: Fixed possible resource leaks; + - putenv() could run out the environment variable area, if + using both file_collection and broker_agent. + - possible handle leak at launching tentacle_client. + * Cleaned source code style. + + Merged from branch 4.x. + 2013-04-01 Sancho Lerena * unix/plugins/inventory: Improved software inventory in RPM format. diff --git a/pandora_agents/win32/ChangeLog b/pandora_agents/win32/ChangeLog index 79bcdea210..88336459ef 100644 --- a/pandora_agents/win32/ChangeLog +++ b/pandora_agents/win32/ChangeLog @@ -1,3 +1,13 @@ +2013-04-11 Hirofumi Kosaka + + * pandora_windows_service.cc: Fixed possible resource leaks; + - putenv() could run out the environment variable area, if + using both file_collection and broker_agent. + - possible handle leak at launching tentacle_client. + * Cleaned source code style. + + Merged from branch 4.x. + 2013-04-04 Koichiro KIKUCHI * windows_service.cc, diff --git a/pandora_agents/win32/pandora_windows_service.cc b/pandora_agents/win32/pandora_windows_service.cc index 239b0fc7cc..a0a2efc121 100644 --- a/pandora_agents/win32/pandora_windows_service.cc +++ b/pandora_agents/win32/pandora_windows_service.cc @@ -374,7 +374,7 @@ Pandora_Windows_Service::getXmlHeader () { string custom_id, url_address, latitude, longitude, altitude, position_description, gis_exec, gis_result; time_t ctime; struct tm *ctime_tm = NULL; - unsigned int pos; + unsigned int pos; // Get agent name agent_name = conf->getValue ("agent_name"); @@ -449,38 +449,38 @@ Pandora_Windows_Service::getXmlHeader () { if(gis_exec != "") { gis_result = getCoordinatesFromGisExec(gis_exec); - if(gis_result != "") { - // Delete carriage return if is provided - pos = gis_result.find("\n"); - if(pos != string::npos) { - gis_result.erase(pos, gis_result.size () - pos); - } - pos = gis_result.find("\r"); - if(pos != string::npos) { - gis_result.erase(pos, gis_result.size () - pos); - } - - // Process the result as "latitude,longitude,altitude" - pandoraDebug ("getCoordinatesFromGisExec: Parsing coordinates %s", gis_result.c_str ()); - pos = gis_result.find(","); - if (pos != string::npos && pos != 0) { - latitude = gis_result; - gis_result = gis_result.substr(pos+1); - latitude.erase(pos, latitude.size () - pos); - pos = gis_result.find(","); - if(pos != string::npos && pos != 0) { - longitude = gis_result; - altitude = gis_result.substr(pos+1); - longitude.erase(pos, longitude.size () - pos); - } + if(gis_result != "") { + // Delete carriage return if is provided + pos = gis_result.find("\n"); + if(pos != string::npos) { + gis_result.erase(pos, gis_result.size () - pos); + } + pos = gis_result.find("\r"); + if(pos != string::npos) { + gis_result.erase(pos, gis_result.size () - pos); + } + + // Process the result as "latitude,longitude,altitude" + pandoraDebug ("getCoordinatesFromGisExec: Parsing coordinates %s", gis_result.c_str ()); + pos = gis_result.find(","); + if (pos != string::npos && pos != 0) { + latitude = gis_result; + gis_result = gis_result.substr(pos+1); + latitude.erase(pos, latitude.size () - pos); + pos = gis_result.find(","); + if(pos != string::npos && pos != 0) { + longitude = gis_result; + altitude = gis_result.substr(pos+1); + longitude.erase(pos, longitude.size () - pos); + } xml += "\" latitude=\""; xml += latitude; xml += "\" longitude=\""; xml += longitude; xml += "\" altitude=\""; xml += altitude; - } - } + } + } } else { latitude = conf->getValue ("latitude"); @@ -527,49 +527,49 @@ Pandora_Windows_Service::getCoordinatesFromGisExec (string gis_exec) string output = ""; int timeout = 30; - /* Set the bInheritHandle flag so pipe handles are inherited. */ - attributes.nLength = sizeof (SECURITY_ATTRIBUTES); - attributes.bInheritHandle = TRUE; - attributes.lpSecurityDescriptor = NULL; - - /* Create a job to kill the child tree if it become zombie */ - /* CAUTION: In order to compile this, WINVER should be defined to 0x0500. - This may need no change, since it was redefined by the - program, but if needed, the macro is defined - in */ - - job = CreateJobObject (&attributes, NULL); - - if (job == NULL) { - pandoraLog ("getCoordinatesFromGisExec: CreateJobObject failed. Err: %d", GetLastError ()); - return ""; - } - - /* Get the handle to the current STDOUT. */ - out = GetStdHandle (STD_OUTPUT_HANDLE); - - if (! CreatePipe (&out_read, &new_stdout, &attributes, 0)) { - pandoraLog ("getCoordinatesFromGisExec: CreatePipe failed. Err: %d", GetLastError ()); - return ""; - } - - /* Ensure the read handle to the pipe for STDOUT is not inherited */ - SetHandleInformation (out_read, HANDLE_FLAG_INHERIT, 0); - + /* Set the bInheritHandle flag so pipe handles are inherited. */ + attributes.nLength = sizeof (SECURITY_ATTRIBUTES); + attributes.bInheritHandle = TRUE; + attributes.lpSecurityDescriptor = NULL; + + /* Create a job to kill the child tree if it become zombie */ + /* CAUTION: In order to compile this, WINVER should be defined to 0x0500. + This may need no change, since it was redefined by the + program, but if needed, the macro is defined + in */ + + job = CreateJobObject (&attributes, NULL); + + if (job == NULL) { + pandoraLog ("getCoordinatesFromGisExec: CreateJobObject failed. Err: %d", GetLastError ()); + return ""; + } + + /* Get the handle to the current STDOUT. */ + out = GetStdHandle (STD_OUTPUT_HANDLE); + + if (! CreatePipe (&out_read, &new_stdout, &attributes, 0)) { + pandoraLog ("getCoordinatesFromGisExec: CreatePipe failed. Err: %d", GetLastError ()); + return ""; + } + + /* Ensure the read handle to the pipe for STDOUT is not inherited */ + SetHandleInformation (out_read, HANDLE_FLAG_INHERIT, 0); + ZeroMemory (&si, sizeof (si)); - GetStartupInfo (&si); - - si.cb = sizeof (si); - si.dwFlags = STARTF_USESTDHANDLES | STARTF_USESHOWWINDOW; - si.wShowWindow = SW_HIDE; - si.hStdError = new_stdout; - si.hStdOutput = new_stdout; - - /* Set up members of the PROCESS_INFORMATION structure. */ + GetStartupInfo (&si); + + si.cb = sizeof (si); + si.dwFlags = STARTF_USESTDHANDLES | STARTF_USESHOWWINDOW; + si.wShowWindow = SW_HIDE; + si.hStdError = new_stdout; + si.hStdOutput = new_stdout; + + /* Set up members of the PROCESS_INFORMATION structure. */ ZeroMemory (&pi, sizeof (pi)); - pandoraDebug ("Executing gis_exec: %s", gis_exec.c_str ()); + pandoraDebug ("Executing gis_exec: %s", gis_exec.c_str ()); /* Create the child process. */ if (CreateProcess (NULL , (CHAR *)gis_exec.c_str (), NULL, NULL, TRUE, @@ -606,7 +606,7 @@ Pandora_Windows_Service::getCoordinatesFromGisExec (string gis_exec) break; } } - + GetExitCodeProcess (pi.hProcess, &retval); if (retval != 0) { @@ -693,6 +693,9 @@ Pandora_Windows_Service::copyTentacleDataFile (string host, CREATE_NO_WINDOW, NULL, NULL, &si, &pi) == 0) { return -1; } + + /* close thread handle, because it won't be used */ + CloseHandle (pi.hThread); /* Timeout */ tentacle_timeout = atoi (conf->getValue ("tentacle_timeout").c_str ()); @@ -704,7 +707,7 @@ Pandora_Windows_Service::copyTentacleDataFile (string host, } if (WaitForSingleObject(pi.hProcess, tentacle_timeout) == WAIT_TIMEOUT) { - TerminateProcess(pi.hThread, STILL_ACTIVE); + TerminateProcess(pi.hProcess, STILL_ACTIVE); CloseHandle (pi.hProcess); return -1; } @@ -1116,7 +1119,7 @@ Pandora_Windows_Service::checkCollections () { int flag, i; char *coll_md5 = NULL, *server_coll_md5 = NULL; - string collection_name, collections_dir, collection_md5, tmp; + string collection_name, collections_dir, collection_path, collection_md5, tmp; string collection_zip, install_dir, temp_dir, dest_dir, path, env; /*Get collections directory*/ @@ -1137,13 +1140,31 @@ Pandora_Windows_Service::checkCollections () { collection_name = conf->getCurrentCollectionName(); if(! conf->getCurrentCollectionVerify() ) { - /*Add the collection directory to the path*/ - tmp = collections_dir + collection_name; - path = getenv ("PATH"); - env = "PATH=" + path + ";" + tmp; - putenv (env.c_str ()); - conf->setCurrentCollectionVerify(); + int found; + /*Add the collection directory to the path (if not exists in %path%)*/ + collection_path = collections_dir + collection_name; + path = getenv ("PATH"); + + /* check if the path is just included in the middle of %path% */ + tmp = collection_path + ";"; /* Added a separator */ + if(path.find(tmp) == string::npos) { + + /* check if the path is the last entry of %path% */ + if( ((found = path.rfind(collection_path)) != string::npos) + && ((found + collection_path.length()) == path.length()) ) + { + /* included already (at the tail of %path%) */ + ; + } + else { + /* it's new ! */ + env = "PATH=" + path + ";" + collection_path; + putenv (env.c_str ()); + } + } + + conf->setCurrentCollectionVerify(); } collection_zip = collection_name+".zip"; @@ -1362,7 +1383,7 @@ Pandora_Windows_Service::checkConfig (string file) { if (tmp.empty ()) { tmp = Pandora_Windows_Info::getSystemName (); } - agent_name = tmp; + agent_name = tmp; /* Error getting agent name */ if (tmp.empty ()) { @@ -1839,9 +1860,9 @@ Pandora_Windows_Service::pandora_run () { for (i=0;i