3

In a game I'm making, folders with text files inside represent world saves, In the load menu of this game I want to have an option to delete a save. I'm using currently this code to try to delete the saves:

hFind = FindFirstFile((dir+"/*").c_str(), &FindFileData);
if (hFind){
    do{
        string s = FindFileData.cFileName;
        if(s.find('.')){//prevents prossesing of "." and ".."
            DeleteFile((dir+"/"+s).c_str());
        }
    }while(FindNextFile(hFind,&FindFileData));
    CloseHandle(hFind);
}
rmdir(dir.c_str());

The only things in these folders are 3 text files so this code should be sufficient, however it isn't. What happens is all the file in the directory are deleted but not the folder, and if I try to delete this folder manually, or edit it in any way while the program is running, windows denies me access. But as soon as I close the game the folder is deleted.

I know the files inside are deleted because I tried the above code with out "rmdir(dir.c_str());" and opened the folder and all the files were gone, also with the above code if I "Delete" the save and then try to load it I there is no world and no inventory, indicating the files have been deleted.

I've tried it with removeDirectory and the same thing happens, It also says it was deleted successfully without any errors.

Why does this happen? How can I avoid this, and get it to work properly?

Any help would be greatly appreciated.


The problem was fixxed with the following code:

hFind = FindFirstFile((dir+"/*").c_str(), &FindFileData);
if (hFind){
    do{
        string s = FindFileData.cFileName;
        if(s.find('.')){//prevents prossesing of "." and ".."
            DeleteFile((dir+"/"+s).c_str());
        }
    }while(FindNextFile(hFind,&FindFileData));
    CloseHandle(hFind);
}
findClose(hFind);
rmdir(dir.c_str());
  • You should stick with the API and use RemoveDirectory. http://msdn.microsoft.com/en-us/library/aa365488%28VS.85%29.aspx If the delete fails, you check the error return code and call GetLastError(). The rmdir() doesn't give you this information (or at least, easily). – PaulMcKenzie Apr 11 '14 at 21:01
  • You should also check whether `DeleteFile` was successful. – R Sahu Apr 11 '14 at 21:07
  • With RemoveDirectory the same thing happens and no error takes place unless I click delete more than once because the program must think the directory is already deleted – MurderFish612 Apr 11 '14 at 21:09
  • While the program is running but after deleting the folder, does the folder still appear to be there even after refreshing the explorer view? – Suedocode Apr 11 '14 at 21:13
  • yes its still there, the system I'm using to display saved worlds is real time so if i went and manually deleted a world I would see it disappear in the program, when I click delete the save stays there, – MurderFish612 Apr 11 '14 at 21:31
  • but yes the folder is still there even after I refresh explorer – MurderFish612 Apr 11 '14 at 21:31
  • @Aggieboy notification – MurderFish612 Apr 11 '14 at 22:00

2 Answers2

4

According to the RemoveDirectory documentation:

RemoveDirectory function marks a directory for deletion on close. Therefore, the directory is not removed until the last handle to the directory is closed.

Probably your program has the directory as its current working directory, or perhaps otherwise still has a handle to it open.

In windows, rmdir is a comparability function that calls the native windows functions, so it will behave the same.

harmic
  • 28,606
  • 5
  • 67
  • 91
  • well I closed the only handle I had open on it, unless there were more that I am unaware of that shouldn't be the problem. How would I know if it was my current working directory, and how would I change it? – MurderFish612 Apr 11 '14 at 22:13
  • You could try GetCurrentDirectory / SetCurrentDirectory http://msdn.microsoft.com/en-us/library/windows/desktop/aa365530(v=vs.85).aspx – harmic Apr 11 '14 at 22:16
  • You can also use ProcessExplorer to see what handles are open http://stackoverflow.com/a/15722/1411457 – harmic Apr 11 '14 at 22:28
  • yes, I changed the current dir with SetCurrentDirectory and the same thing is happening – MurderFish612 Apr 11 '14 at 22:29
  • Is there like a clearHandles function? that would be helpfull – MurderFish612 Apr 11 '14 at 22:30
  • wait set currentDirectory failed with a code of 123 – MurderFish612 Apr 11 '14 at 22:37
  • Seems you are not passing in correct paths. 123 = `The filename, directory name, or volume label syntax is incorrect.`. 3=`Path not found` – harmic Apr 11 '14 at 22:50
  • ok, so i put 'if(!SetCurrentDirectory("C:")) cout << GetLastError() << endl;' after RemoveDirectory same thing happens, no errors – MurderFish612 Apr 11 '14 at 22:59
  • 1
    ok, I got it to work, i was reading this i just found http://stackoverflow.com/questions/10832251/function-to-remove-directory-removes-it-only-after-debug-finishes-c and tried adding 'FindClose( hFind );' right before RemoveDirectory and it worked sorry I wasted your time and thanks for the help – MurderFish612 Apr 11 '14 at 23:05
1

The root problem is that the code called CloseHandle instead of FindClose on the handle returned by FindFirstFile.

But the code has several more bugs. In the interest of helping future visitors here, this is the corrected code.

HANDLE hFind = FindFirstFile((dir + "\\*").c_str(), &FindFileData);  // See 1 below
if (hFind != INVALID_HANDLE_VALUE) {  // 2
    do {
        if ((FindFileData.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) == 0) {  // 3
            const std::string s = dir + "\\" + FindFileData.cFileName;
            DeleteFile(s.c_str());
        }
    } while (FindNextFile(hFind, &FindFileData));
    // 4
    FindClose(hFind);  // 5
}
RemoveDirectory(dir.c_str());  // 6
  1. Windows paths use \ rather than / as separators. Many of the APIs will accept either, but eventually you'll encounter one that doesn't, so it's best to use the correct one consistently.

  2. FindFirstFile returns INVALID_HANDLE_VALUE (not NULL) upon failure. Since INVALID_HANDLE_VALUE is non-zero, you cannot simply test if (hFile) { ... }.

  3. The API enumerates files and directories. The old code was trying to filter out the . and .. directories incorrectly, which could have caused it to skip some files and to attempt to use DeleteFile on other directories. It's simpler (and easier-to-understand) to skip all directories.

  4. Don't call CloseHandle on the handle returned by FindFirstFile.

  5. Do call FindClose on the handle returned by FindFirstFile, but do so only in the case you got a valid handle from FindFirstFile.

  6. As long as you're using Windows-specific APIs, you may as well use them consistently and not mix with library wrappers like rmdir. The library wrappers sometimes introduce surprising limitations or behavior, though I think rmdir would work correctly in this instance.

This still leaves a significant problem: It doesn't handle Unicode in the paths (and it requires you to compile for ANSI which limits Unicode handling in other parts of the project).

Adrian McCarthy
  • 45,555
  • 16
  • 123
  • 175