I recently worked on a bug in which a product developed by my employer was no longer finding user settings files. Everything worked correctly in a prior version of the product but failed in the current version.
The following is a simplified version of the old code.
BOOL GetUserProfilePath(LPWSTR profilePathName) { if (profilePathName == nullptr) return FALSE; profilePathName[0] = static_cast<wchar_t>(0); std::wstring userPath = GetUserPath(); if (userPath.empty()) return FALSE; TCHAR tempPath[MAX_PATH]; GetProgramPath(tempPath); ::PathAppend(tempPath, userPath.c_str()); ::PathCanonicalize(profilePathName, tempPath); if (profilePathName[0]) { return TRUE; } return FALSE; }
The following is a simplified version of the new code.
BOOL GetUserProfilePath(std::wstring& profilePathName) { profilePathName.clear(); std::wstring userPath = GetUserPath(); if (userPath.empty()) return FALSE; wstring tempPath; GetProgramPath(tempPath); Path::Append(tempPath, userPath); wchar_t temp[MAX_PATH]; PathCanonicalize(temp, tempPath.c_str()); profilePathName = temp; if (!profilePathName.empty()) { return TRUE; } return FALSE; }
At first glance these two implementations appear to be equivalent.
Now, here are a few additional details that reveal why they are not in any way equivalent.
First, due to a previously unknown bug, the GetUserPath function returned a complete path, not a relative path. It was, in fact, the program path. Thus if the program path was “c:\MyApp,” then the value returned by the GetUserPath function was “c:\MyApp.”
Second, the GetProgramPath function also obtains the program path, thus in this example its value would also be “c:\MyApp.”
Thus, when the ::PathAppend Win32 API function was being called it was being asked to append a full path onto another full path. Microsoft chose to, in their infinite wisdom, attempt to protect you from this mistake by trying to *just do the right thing* by generating a valid path despite your mistake. Thus, the bug in the GetUserPath function was harmless.
The person who wrote the Path::Append function was not aware of this. The following was their implementation of the function.
namespace Path { // Various details left out. bool Append(std::wstring& dest, const std::wstring& source) { Path::AddBackslash(dest); dest += source; return !dest.empty(); } }
Therefore, the end result of this change was that the otherwise harmless bug in the GetUserPath function suddenly became a big deal. Before this change, the GetUserProfilePath function returned the string “c:\MyApp” regardless of the bug in the GetUserPath function. After this change the GetUserProfilePath function returned the string “c:\MyApp\c:\MyApp,” which is an invalid path.
I was asked to fix this with the smallest change possible. The GetUserPath function happens to be part of a deprecated component that we are trying to phase out. Therefore, I was not allowed to touch it. As a result, I chose to fix it by simply modifying the Path::Append function so that it uses the ::PathAppend Win32 API function instead of attempting to do the work itself.
I am sharing this in the hopes that it could spare you the difficulties I had over the three days it took me to diagnose and fix this bug.