PeterTh / gedosato

The Generic DownSampling Tool
GNU General Public License v3.0
462 stars 166 forks source link

`getSystemDllName (...)` makes poor choice of log text regarding DLL paths. #330

Closed Kaldaien closed 8 years ago

Kaldaien commented 8 years ago

On line 733 of detouring.cpp, we have:

string getSystemDllName(const string& name) { 
  char sysdir[MAX_PATH]; 
  GetSystemDirectory(sysdir, MAX_PATH); 
  string fullPath = string(sysdir) + "\\" + name; 
  SDLOG(28, "Full dll path: %s\n", fullPath.c_str()); 
  return fullPath; 
} 

This will always return the host's native DLL directory (e.g. 64-bit DLLs on a 64-bit system and 32-bit DLLs on a 32-bit system). If the software is 32-bit on a 64-bit operating system, this "SystemDllName" seems rather not the "Full dll path" as the log claims - the DLL appropriate for the running architecture will be found in the Wow64 directory rather than the location listed.

You can open that file from a 32-bit piece of software and Windows is going to silently redirect everything to SysWow64. But this doesn't feel right. If you use that file name in a log, there's no way to distinguish it from the 64-bit version - that is NOT its fully-qualified name.


Logically, I would expect to see something more like this:

string getSystemDllName(const string& name) { 
  BOOL bWow64 = FALSE;
  IsWow64Process (GetCurrentProcess (), &bWow64);

  char sysdir[MAX_PATH];

  if (bWow64)
    GetSystemWow64Directory(sysdir, MAX_PATH);  // SysWOW64 (generally speaking)
  else
    GetSystemDirectory(sysdir, MAX_PATH);       // System32 (generally speaking)

  string fullPath = string(sysdir) + "\\" + name; 
  SDLOG(28, "Full dll path: %s\n", fullPath.c_str()); 
  return fullPath; 
}

This properly handles the very common case where a 32-bit piece of software is running on a 64-bit OS. If you are going to fully-qualify these DLLs, it would make sense to do so without relying on FS redirection.

PeterTh commented 8 years ago

Good point.

PeterTh commented 8 years ago

Fixed in the update pushed today.