Why does SHFileOperationW create folder instead of move file?

Question

I have created own "Move" function based on SHFileOperationW like this:

int MoveItem(string strSourcePath, string strDestinationDirectory, string strNewName, bool bAllowOverwrite, bool bCreateDestinationDirectoryIfNotExist)
{
    int intPositionOfLastSlash;
    int intResult = 0;

    SHFILEOPSTRUCTW sfosMove;

    string strNewPath;
    string strOldName;

    wchar_t a_wcNewPath[MAX_PATH] = {0};
    wchar_t a_wcSourcePath[MAX_PATH] = {0};

    wstring wsNewPath;
    wstring wsSourcePath;

    if((IsFileExist(strSourcePath) == false) && (IsFolderExist(strSourcePath) == false))
    {
        intResult = 0x7C;
    }

    else
    {
        if(IsFolderExist(strDestinationDirectory) == false)
        {
            if(bCreateDestinationDirectoryIfNotExist == false)
            {
                intResult = 0x7C;
            }
        }
    }

    if(intResult == 0)
    {
        intPositionOfLastSlash = strSourcePath.find_last_of("\\");
        strOldName = strSourcePath.substr(intPositionOfLastSlash + 1);

        if(strNewName == "")
        {
            strNewPath = strDestinationDirectory + "\\" + strOldName;
        }

        else
        {
            strNewPath = strDestinationDirectory + "\\" + strNewName;
        }

        if(bAllowOverwrite == false)
        {
            if(IsFileExist(strNewPath) == true)
            {
                intResult = 0x7E;
            }

            else if(IsFolderExist(strNewPath) == true)
            {
                intResult = 0x80;
            }
        }
    }

    if(intResult == 0)
    {
        /*- Source Path -*/
        wsSourcePath = Utf8StringToWstring(strSourcePath);

        StrCpyW(a_wcSourcePath, wsSourcePath.c_str());

        CopyMemory(a_wcSourcePath + lstrlenW(a_wcSourcePath), L"\0\0", 2);
        /*---------------*/

        /*- New Path -*/
        wsNewPath = Utf8StringToWstring(strNewPath);

        StrCpyW(a_wcNewPath, wsNewPath.c_str());

        CopyMemory(a_wcNewPath + lstrlenW(a_wcNewPath), L"\0\0", 2);
        /*-------------------------*/

        /*- Move Item -*/
        sfosMove.fFlags = FOF_NOCONFIRMATION | FOF_NOERRORUI | FOF_SILENT;
        sfosMove.pFrom = a_wcSourcePath;
        sfosMove.pTo = a_wcNewPath;
        sfosMove.wFunc = FO_MOVE;

        intResult = SHFileOperationW(&sfosMove);
        /*-------------*/
    }

    return intResult;
}

But The problem is when I use this function repeatedly, only first file is moved. The function creates folders with name of files.

For example:

First file: abc.txt (This file is moved.) Second file: def.txt (This file is not moved. The function creates a folder named "def.txt")

Example usage creates the problem:

// "example_folder" is already present in "D:" drive.
int intResult;

intResult = MoveItem("C:\\ProgramData\\abc.txt", "D:\\example_folder", "", true, false);

if(intResult == 0)
{
     intResult = MoveItem("C:\\ProgramData\\def.txt", "D:\\example_folder", "", true, false);     
}   

Answer 1

🛑☠️ Potential buffer overruns in several places here.

1. You should be allocating filename buffers with size [MAX_PATH + 1], because they must have enough room to hold MAX_PATH characters and the terminating double-null.
2. You are only copying two bytes to the end of both your source and your destination paths, when your intent is to copy four. L"\0\0" is two wide characters, which is four bytes.

CopyMemory(a_wcSourcePath + lstrlenW(a_wcSourcePath), L"\0\0", 2);

Will this matter? Practically speaking, probably not, as you did initialize the buffer to all-zeroes. But it's a recipe for potential disaster.

3. You are copying to a buffer that may not have room for the characters you're copying. (It would usually work out, since presumably your source path is not longer than MAX_PATH, but you're dealing with conversions as well, from UTF-8 to UTF-16; for safety's sake, you should use StringCchCopy() or similar.)

    wsSourcePath = Utf8StringToWstring(strSourcePath);

    StrCpyW(a_wcSourcePath, wsSourcePath.c_str());

4. You are not initializing the SHFILEOPSTRUCTW struct, so you have no way of knowing what sort of data is sitting in its various parts. Like this:

SHFILEOPSTRUCTW sfosMove{};

5. Will fixing all of those issues fix your problem? Maybe, but I don't know. (You have several functions in your pasted code that are not defined, for one thing.) When I run this code, more or less as you have it there, with the problems I've noted here fixed, it seems to work fine.

This whole answer might get downgraded because it's not definitively an "answer," but hopefully it saves you from some trouble down the line.