Part 2: The most common mistakes in using SHFileOperation For my second installment, I thought it would be helpful to point out the most common coding mistakes I've seen with SHFileOperation.
1. Always double-null terminate your paths
This is the most common error so I am listing it first. The SHFILEOPSTRUCT takes a buffer for the source (pFrom) and destination (pTo) path(s). These buffers can be composed of multiple null-delimited paths that are ended with a double null terminator. MSDN calls this out below for the pFrom member of SHFILEOPSTRUCT:
Address of a buffer to specify one or more source file names. These names must be fully qualified paths. Standard Microsoft MS-DOS wild cards, such as "*", are permitted in the file-name position. Although this member is declared as a null-terminated string, it is used as a buffer to hold multiple file names. Each file name must be terminated by a single NULL character. An additional NULL character must be appended to the end of the final name to indicate the end of pFrom.
The pTo member is similarly documented.
Therefore, if "c:\windows\*" was the path being placed in the buffer, it would need to be double-null terminated as shown below:
What developers fail to do is ensure their paths are terminated properly. Here is an example of what not to do:
// Declare a string pointer for our path - THIS IS WRONG!
LPTSTR pszSource = L"C:\\Windows\\*";
...
// Set the SHFILEOPSTRUCT
shfo.pFrom = pszSource;
In memory this looks like:
When this code is run, it is possible that the pszSource string will be double-null terminated by accident - depending on where the string aligns in memory. Yet, the path is only single-null terminated. So eventually, the SHFileOperation API will assume that after encountering only a single NULL at the end of this string, it will read beyond it until it encounters a double-null. The potential is a buffer overrun and a security exploit. Even scarier (although less likely) would be if the buffer sits on the heap next to another valid path. Suddenly, your call to SHFileOperation just deleted more than you had anticipated!
A bug in an unnamed, non-MS product hit this during beta testing of Vista. The caller used SHFileOperation to delete contents under its application's folder. Unfortunately, they did not double-null terminate their buffer. Right after their path was a "*.*" which was interpreted by SHFileOperation to be the current directory - which happened to be c:\windows\system32!!! At the timeframe of Beta2 for Vista, most manifest work to properly use Windows File Protection (WFP) was not completed. So the application managed to permanently delete much of the contents under the system32 directory - causing the OS to fail to boot after restart. This was not a problem in XP since WFP prevented the files from being deleted. Since then, those files under the system32 directory are now protected against this.
The correct double-null terminated path declaration should be:
LPTSTR pszSource = L"C:\\Windows\\*\0";
It would also be a good time to point out that you should take these double-nulls into account when determining the size of your string buffer to store paths. The maximum length path (in characters) that can be used by the shell is MAX_PATH (defined as 260). Therefore, you should create buffers that you will pass to SHFILEOPSTRUCT to be of length MAX_PATH + 1 to account for these NULLs.
TCHAR szFoo[MAX_PATH + 1] = {0} // Initialize to all zero
2. Always specify full paths
Be explicit. Always ensure all your paths are full paths. Otherwise, this will lead to unpredictable results.
Many developers are confused in how the SHFileOperation API handles partial or incomplete paths:
- Leaving off the path to a file or folder DOES NOT mean it is off the root of the current directory.
- The path environment variable IS NOT used to find a valid path.
- Do NOT depend on SHFileOperation to use the current directory with partial paths. Since the current directory is process-wide, it can be changed from another thread while the operation is executing - leading to unpredictable results.
3. Always check the fAnyOperationsAborted flag
While the API can return 0 indicating success, the caller should check the fAnyOperationsAborted flag as well. This is set to TRUE if the user has cancelled the operation via the UI the API presents (progress/confirmation/error dialogs) or if the operation was silently cancelled (if FOF_NOERRORUI or FOF_NOCONFIRMATION flags were used).
4. Use the FOF_WANTNAMEMAPPING flag correctly
If you want to use the name mapping feature of SHFileOperation, be sure you clean up after yourself. The FOF_WANTNAMEMAPPING flag (used in conjunction with the FOF_RENAMEONCOLLISION) will create a handle to an array of SHNAMEMAPPING structures that need to be cleaned up in a subsequent call to SHFreeNameMappings. Without such a call, the memory will be leaked by your application.
5. Do not use GetLastError to determine if an error occurred
As was mentioned in the previous post on SHFileOperation, the API returns some error codes that do not map to what actually went wrong. To get some insight into what the real error is, some developers call GetLastError to retrieve the last error code set. This is incorrect. Nowhere in the MSDN documentation does it say that SHFileOperation will clear or set the last error. It is true that sometimes an API that is called from inside SHFileOperation may set the last error. Yet, in most cases the error will have been set from before the SHFileOperation call and the caller will not know for sure if it is the result of the operation that it performed.
6. Only use wildcards in paths where they are valid
The SHFileOperation API does allow the use of wild cards such as "*" - but only in specific places: in the pFrom buffer, within the file-name position. Using wild cards in any other place in the pFrom buffer or anywhere in the pTo buffer will lead to unpredictable results.
Also note, you cannot use wildcards for rename operations.
7. Do not use \\?\... in your paths
Some of the Windows APIs allow you to pass paths that exceed MAX_PATH using a "\\?\" prefix. SHFileOperation is NOT one of them. In fact, you should not be passing these paths to any Shell APIs. Prefixing your paths as such will lead to SHFileOperation returning error 124 (DE_INVALIDFILES) or other unpredictable results.
I would have liked to come up with 10 issues, but I'm all out of good ones. It just seems to be more official and Letterman-ish to have 10. So if you can come up with any more, please post them in the comments section.