Only take into account processes that have our plugin loaded

installer/src/installer-lib/process.cpp

Index: installer/src/installer-lib/process.cpp
===================================================================
--- a/installer/src/installer-lib/process.cpp
+++ b/installer/src/installer-lib/process.cpp
@@ -31,7 +31,11 @@
 {
   if ( handle == INVALID_HANDLE_VALUE )
   {
-    throw std::runtime_error( "Invalid handle" ) ;
+    // TODO: This code really deserves use of a proper exception class that packages 
+    // Windows API errors consistently.
+    char tmp[256];
+	sprintf(tmp, "Invalid handle. Last error: %d", GetLastError());

[Wladimir Palant, 2014/03/28 07:30:17]

sprintf() is deprecated, please use sprintf_s() instead.

Also a nit: Please use spaces for indentation.

+    throw std::runtime_error( tmp ) ;
   }
 }
 
@@ -47,12 +51,29 @@
   return 0 == wcsncmpi( process.szExeFile, name, length ) ;
 }
 
+
 //-------------------------------------------------------
-// process_by_any_exe_name_CI
+// process_by_any_exe_with_any_module
 //-------------------------------------------------------

[Wladimir Palant, 2014/03/28 07:30:17]

I guess I am missing the point of having of this comment - what value does it
add?

-bool process_by_any_exe_name_CI::operator()( const PROCESSENTRY32W & process )
+bool process_by_any_exe_with_any_module::operator()( const PROCESSENTRY32W & process )
 {
-  return names.find( process.szExeFile ) != names.end() ;
+  if ( processNames.find( process.szExeFile ) != processNames.end() )
+  {
+      if (moduleNames.empty())
+		  return true;
+
+	  ModulesSnapshot ms( process.th32ProcessID );
+	  MODULEENTRY32* me = ms.begin();
+	  while ( me != ms.end() )
+	  {
+		  if (moduleNames.find( me->szModule ) != moduleNames.end())
+		  {
+			  return true;
+		  }
+		  me = ms.next();
+	  }

[Wladimir Palant, 2014/03/28 07:30:17]

Nit: Please use spaces for indentation.

+  }
+  return false;
 }
 
 //-------------------------------------------------------
@@ -162,6 +183,26 @@
 
 
 //-------------------------------------------------------
+// ModulesSnapshot
+//-------------------------------------------------------
+ModulesSnapshot::ModulesSnapshot(DWORD processId)
+  : handle( ::CreateToolhelp32Snapshot( TH32CS_SNAPMODULE | TH32CS_SNAPMODULE32, processId ) )

[Wladimir Palant, 2014/03/28 07:30:17]

Style nit: That's rather unusual style in this file - spaces after opening
parenthesis and before closing parenthesis, usually we don't do that in our
code. Please remove them in your changes, it needs to be fixed for the rest of
the file as well eventually.

+{
+  module.dwSize = sizeof( MODULEENTRY32 ) ;
+}
+
+MODULEENTRY32W * ModulesSnapshot::begin()
+{
+  return ::Module32FirstW( handle, & module ) ? ( & module ) : 0 ;
+}
+
+MODULEENTRY32W * ModulesSnapshot::next()
+{
+  return ::Module32NextW( handle, & module ) ? ( & module ) : 0 ;
+}

[Wladimir Palant, 2014/03/28 07:30:17]

This fakes a C++ iterator interface but does so incompletely - the iterator
should be a reference, next() and end() have to be methods of the iterator. IMHO
in that case it is better to not even pretend being an iterator. These methods
can be called first() and next() and the caller can simply check whether it gets
NULL.

[Eric, 2014/03/28 12:06:00]

Yep. It really ought be a proper iterator class, but that can't be done. It's a
wrapper around a Windows system call with state implicitly embedded in a handle.
There's another one just like it the Snapshot class, which uses the same system
call to retrieve all processes in the system; this one retrieves all modules for
a process.
I would have done exactly that had it been possible. The very short explanation:
iterators must be copyable.
Using the iterator naming convention makes the loops like just like other ones
in C++. It's more indicative when reading the code what's going on, assuming one
is used to reading C++ code.

[Oleksandr, 2014/03/28 12:48:35]

I actually like the way it is implemented. The iterator in this case is "handle"
and begin(), next() and end() are all there and can be considered methods of the
"handle". I would like to leave it as is. Can you please give an example of why
this isn't a good approach?

On 2014/03/28 07:30:17, Wladimir Palant wrote:

[Wladimir Palant, 2014/03/28 13:55:24]

This is simply misleading, one looks at the loop and expects a real iterator.
The difference is only obvious at the second glance, this invites overlooking
issues or making mistakes when using this interface in own code.

[Oleksandr, 2014/03/28 14:28:16]

Fixed in Patchset 4
On 2014/03/28 13:55:24, Wladimir Palant wrote:

+
+
+//-------------------------------------------------------
 // send_message, send_endsession_messages
 //-------------------------------------------------------
 /**