Windows C++: Folder moving Access Denied error

Question

The code below is a snippet from my project that among other things:

1. Gets a collection of folder contents from a specified local folder (The Upload folder)
2. Copies the contained files to a remote network folder. If there are sub folders their files are copied, but the folder structure is not preserved.
3. Moves the original local folders to a different local location (The Archive folder)

The trouble I'm having is in the move operation. If the folder to be moved contains one or more sub folders, the move fails with an 'access denied' error. I've narrowed the cause down to a previous function call that generates the list of files for copying. I've highlighted this call in the code below. It's in main (see first call to getFolderItemsRecursive()). I don't understand why this recursive function is creating issues. Its as if by collecting the information its in someway marking something as 'in use'. Note that I'm not actually copying the files as I commented these lines out already. So that's not it.

To run the code below you'll need to create a couple of folders somewhere and enter the locations into the code. The code assumes c:_UploadTests and c:_archivedtests. To reproduce the issue create at least one subfolder, with its own subfolder, in _UploadTest. E.g. c:_UploadTests\Foo\Bar\

#include "windows.h"
#include "stdafx.h"
#include <cstdint>
#include <vector>

#define RECURSION_DEPTH_NON 0
#define RECURSION_DEPTH_FULL -1

// Object needed bacause WIN32_FIND_DATAA doesn't contain file path data and we need this with each object.
typedef struct fileObject{
    std::string path;
    WIN32_FIND_DATAA metaData;
} fileObject;

void getFolderItems(std::vector<WIN32_FIND_DATAA>& output, const std::string& path) {
    WIN32_FIND_DATAA findfiledata;
    HANDLE hFind = INVALID_HANDLE_VALUE;

    char fullpath[MAX_PATH];
    GetFullPathNameA(path.c_str(), MAX_PATH, fullpath, 0);
    std::string fp(fullpath);

    hFind = FindFirstFileA((fp + "\\*").c_str(), &findfiledata);
    if (hFind != INVALID_HANDLE_VALUE) {
        do {
            switch (findfiledata.dwFileAttributes) {
            case FILE_ATTRIBUTE_DIRECTORY:                  // Its a directory
                if (findfiledata.cFileName[0] != '.')
                    output.push_back(findfiledata);
                break;
            case FILE_ATTRIBUTE_NORMAL:                     // Its a file
            case FILE_ATTRIBUTE_ARCHIVE:
            case FILE_ATTRIBUTE_COMPRESSED:
                output.push_back(findfiledata);
                break;
            }
        } while (FindNextFileA(hFind, &findfiledata) != 0);
    }
}

/// Gets a list of directory contents and their sub folders under a specified path
/// @param[out] output Empty vector to be filled with result
/// @param[in]  path   Input path, may be a relative path from working dir
/// @param[in]  depth  Max depth of recursion relative to 'path'. -1 = full recursion, 0 = non, 1 = allow one level down, etc
void getFolderItemsRecursive(std::vector<fileObject>& output, const std::string& path, int depth) {
    std::vector<WIN32_FIND_DATAA> thisLvl;
    fileObject fileObj;

    // Get all the items from this level folder
    getFolderItems(thisLvl, path);

    // Loop through the items and dive deeper into any subfolders
    for (std::vector<WIN32_FIND_DATAA>::iterator i = thisLvl.begin(); i != thisLvl.end(); ++i) {
        // Add the current folder to the object list
        fileObj.metaData = *i;
        fileObj.path = path;
        output.push_back(fileObj);

        if (fileObj.metaData.dwFileAttributes == FILE_ATTRIBUTE_DIRECTORY) {
            if (depth == RECURSION_DEPTH_FULL)
                getFolderItemsRecursive(output, path + std::string("\\") + fileObj.metaData.cFileName, RECURSION_DEPTH_FULL);
            else if (depth > 0)
                getFolderItemsRecursive(output, path + std::string("\\") + fileObj.metaData.cFileName, depth--);
        }
    }
}


int main(int argc, char** argv) {
    // manually create the list of upload folders
    std::vector<std::string> uploadFoldersVector;
    uploadFoldersVector.push_back("c:\\_UploadTests");

    // For each upload folder, get a list of sub folders and loop through them copying out the files.
    for (uint8_t i = 0; i < uploadFoldersVector.size(); i++) {
        std::string srcPath;
        std::string dstPath;
        BYTE flags;

        // Get a list of this folders contents
        std::vector<fileObject> uploadFiles;

        /*********************
        The function below seems to be the culprit when called with RECURSION_DEPTH_FULL - look in all subfolders and their children.
        A similar call but with RECURSION_DEPTH_NON doesn't cause any issues.
        **********************/
        getFolderItemsRecursive(uploadFiles, uploadFoldersVector[0], RECURSION_DEPTH_FULL);

        // For each file object, copy it to the network archive.
            // Removed for StackOverflow for simplicity - adds nothing to the problem definition

        // Finally move this folder into the local archive folder
            // Get a list of immediate sub folders
            uploadFiles.clear();
            getFolderItemsRecursive(uploadFiles, uploadFoldersVector[0], RECURSION_DEPTH_NON);

            flags = MOVEFILE_COPY_ALLOWED | MOVEFILE_REPLACE_EXISTING;

            for (uint8_t j = 0; j < uploadFiles.size(); j++) {
                srcPath = uploadFoldersVector[0] + "\\" + uploadFiles[j].metaData.cFileName;
                dstPath = "c:\\_archivedtests" + (std::string)uploadFiles[j].metaData.cFileName;

                if (!MoveFileExA(srcPath.c_str(), dstPath.c_str(), flags)) {
                    fprintf(stderr, "Error moving folder %s to local archive. \n\tWindows returned code: %ld\n", srcPath.c_str(), GetLastError());
                }
            }
    }

    getchar();
    return 0;
}

Answer 1

getFolderItems() is not calling FindClose() after the iteration loop is finished, so you are leaking open handles to folders that are being iterated, that is why you see them as "in use".

if (hFind != INVALID_HANDLE_VALUE) {
    do {
        ...
    } while (FindNextFileA(hFind, &findfiledata) != 0);
    FindClose(hFind); // <-- ADD THIS!
}

getFolderItems() also has other bugs, too:

• if (findfiledata.cFileName[0] != '.') is the wrong check to use, as there can be folders present other than . and .. that also begin with a ., so you need to use this kind of check instead:

  if ((strcmp(findfiledata.cFileName, ".") != 0) && (strcmp(findfiledata.cFileName, "..") != 0))
  

• using a switch to check for attributes is wrong, as entries may have multiple attributes present. You need to use the bitwise AND (&) operator to check for specific attributes:

  if (findfiledata.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)
  {
      // Its a directory
      ...
  }
  else
  {
      // Its a file
      if (findfiledata.dwFileAttributes & (FILE_ATTRIBUTE_NORMAL | FILE_ATTRIBUTE_ARCHIVE | FILE_ATTRIBUTE_COMPRESSED))
          ...
  }
  

Also, instead of moving individual files with MoveFileExA(), consider using SHFileOperation() or IFileOperation::MoveItems() instead so you can move multiple files in a single operation.