0

I'm having a problem when deleting folders that are on a different partition (E:/) from my software. I can delete files, using the DeleteFile function, but I'm not able to delete a folder using the code below:

function RemoveDirectory(strDir : String) : Boolean;
var
  SearchRec : TSearchRec;
  strFile   : String;
  nResult   : Integer;
begin
  try
    Result := false;

    nResult := FindFirst(strDir + '*', faAnyFile, SearchRec);

    while (nResult = 0) do
      begin
        if (SearchRec.Name <> '.') and (SearchRec.Name <> '..') then
          begin
            strFile := strDir +  SearchRec.Name;

            if FileExists(strFile) then
              DeleteFile(strFile)
            else if DirectoryExists(strFile) then
              RemoveDirectory(strFile);
          end;
        nResult := FindNext(SearchRec);
      end;

    Result := RemoveDir(strDir);
  finally
    FindClose(SearchRec);
  end;
end;

With this code I can delete folders that are on the same partition from my software. Somebody knows what's going on? Is it because it's on a different partition?

  • What happens when you step through the code in the debugger? Do you check any error/return values or get any exceptions raised? These will all help you solve your problem. – Andy_D Jul 31 '14 at 12:43
  • It doesn't matter whether or not the files are on the same mapped drive or not. Something is stopping you delete files/directories. The fact that you did not check for errors is a worrying sign. If you want to know what's gone wrong, why did you neglect to check for errors? And why use `FileExists` and `DirectoryExists`? `SearchRec.Attr` tells you that information. Your `try/finally` is all wrong too. The `try` is too soon. The next step for you is to add error checking. – David Heffernan Jul 31 '14 at 12:49
  • Can you delete these directories manually? – Jan Doggen Jul 31 '14 at 14:47
  • Ok, I added error checking and got the error 32, which indicates that some process is using it. But if I put a Sleep before this function, and go by myself I can delete the folder manually. So I think that what is holding the folder is the function itself. PS: All the files (does not have folders inside) inside that directory were successfully deleted. – Alexandre Severo Jul 31 '14 at 14:58
  • Quite possibly anti-virus. You might contemplate `SHFileOperation` here – David Heffernan Jul 31 '14 at 14:59
  • 3
    I notice that you call `Result := RemoveDir(strDir)` BEFORE `FindClose(SearchRec);`. I am not going to say for sure that is your problem, (and you should heed the warnings about error handling) but it still strikes me as back-to-front. – Hugh Jones Jul 31 '14 at 15:43
  • SHFileOperation worked perfectly. About @HughJones tip, I think it could be the problem, because as the handler is searching for files inside that directory, it blocks any changes to the folder itself. But as I said before, I can delete the folder if it is on the same partition (or on a pc that has a different anti-virus), so I think the David Heffernan's guess is right. Thank you all, guys! – Alexandre Severo Aug 01 '14 at 12:24
  • It sounds like you have ruled out the partition suggestion already; what makes the difference is a different anti-virus. (btw, I doubt David 'guessed') – Hugh Jones Aug 01 '14 at 12:29

2 Answers2

4

You are trying to remove directories while you still have open search handles. Since this is a recursive function, if the directory hierarchy is deep, you would have multiple search handles open at a time and that is a lot of system resources being used when the deeper folders are reached.

It is better to collect the immediate subfolders into a temp list, then you can close the current search handle before iterating that list. This way, there is ever only 1 search handle active at a time, and there is no search handle active when each folder is actually being deleted.

Try this:

function RemoveDirectory(strDir : String) : Boolean;
var
  SearchRec : TSearchRec;
  nResult,i : Integer;
  SubFolders: TStringList;
begin
  SubFolders := nil;
  try
    strDir := IncludeTrailingPathDelimiter(strDir);

    nResult := FindFirst(strDir + '*', faAnyFile, SearchRec);
    if (nResult = 0) then
    try
      repeat
        if (SearchRec.Attr and faDirectory) = 0 then
          DeleteFile(strDir + SearchRec.Name)
        else
        begin
          if (SearchRec.Name <> '.') and (SearchRec.Name <> '..') then
          begin
            if not Assigned(SubFolders) then SubFolders := TStringList.Create;
            SubFolders.Add(strDir + SearchRec.Name);
          end;
        end;
      until FindNext(SearchRec) <> 0;
    finally
      FindClose(SearchRec);
    end;

    if Assigned(SubFolders) then
    begin
      for i := 0 to SubFolders.Count-1 do
        RemoveDirectory(SubFolders[i]);
    end;
  finally
    SubFolders.Free;
  end;

  Result := RemoveDir(strDir);
end;

If this still fails, then someone else outside of your app/loop is actually using the directories, and you can use a tool like SysInternals Process Explorer to check that.

Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
  • 1
    Could you explain why the original code failed, and why this code will not fail? – David Heffernan Jul 31 '14 at 16:06
  • I don't see that you need the string list. Also, are you sure that a directory cannot be deleted if the is an open search rec for it? – David Heffernan Jul 31 '14 at 20:41
  • The primarily purpose of using the StringList is to not have multiple search handles open at the same time. Since this is a recursive function, if the directory hierarchy is deep, that is a lot of system resources being used by the time the deeper folders are reached. That is why it is better to collect the immediate subfolders into a temp list, close the current search handle, and then iterate the subfolders. That way, only 1 search handle is ever active at a time, and there is no search handle active when each folder is actually being deleted. – Remy Lebeau Jul 31 '14 at 20:48
  • That makes sense. I wish that info was in the answer. And I suspect you are right that open search handles can be a problem. Although it seems not always. – David Heffernan Jul 31 '14 at 20:50
  • I added the info to the answer. – Remy Lebeau Jul 31 '14 at 21:13
  • One final comment. You don't check for errors apart from once. The top level call to RemoveDir. – David Heffernan Jul 31 '14 at 21:16
  • If the subitems fail to delete than `RemoveDir()` will fail with a "not empty" error, so there is no real point in doing error checking on the subitem deletes unless you need to log them individually. Of course, the error checking on the search itself could always be enhanced to report failures when `FindFirst()` and `FindNext()` return any errors other than `ERROR_FILE_NOT_FOUND` and `ERROR_NO_MORE_FILES`, respectively. – Remy Lebeau Jul 31 '14 at 21:28
  • It would be nice to know which specific file cannot be deleted. – David Heffernan Jul 31 '14 at 21:39
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/58455/discussion-between-remy-lebeau-and-david-heffernan). – Remy Lebeau Jul 31 '14 at 22:54
  • 1
    @Remy - is it certainly a problem removing the folder before closing the search handle? I was not sure. – Hugh Jones Aug 01 '14 at 08:07
-2

DeleteFile() is boolean function and you can receive only information was it successful or not. If you want more details return to the plain old Erase():

var f: file;

begin
  AssignFile(f,strFile);
  Erase(f);
end;

Here, if Erase() is not completed, an exception will be raised and you can receive more info, especially in the debugging phase.

LHristov
  • 1,103
  • 7
  • 16
  • 1
    -1 The Windows `DeleteFile` API is the correct way to delete a file in Delphi 6. The function of the same name in `SysUtils` is implemented as nothing more than a call to the Win32 API function. If the function returns false then you call `GetLastError` to find out more information. This is detailed clearly in the documentation: http://msdn.microsoft.com/en-gb/library/windows/desktop/aa363915.aspx Wrap it up with a call to `RaiseLastOSError` or `Win32Check`. – David Heffernan Jul 31 '14 at 14:45
  • 1
    Of course, `Erase` is implemented on top of the OS. And so it is nothing more than a call to Win32 `DeleteFile` followed by a call to `GetLastError` in case of failure. Recommending legacy Pascal I/O is not the way forward. – David Heffernan Jul 31 '14 at 14:51
  • **Modified** legacy code, now made multiplatform. And this is the correct way nowadays to find out why you cannot delete a file on Mac or Android. – LHristov Jul 31 '14 at 14:56
  • It certainly is not. The x-plat file handling utilities are found in the IOUtils unit. – David Heffernan Jul 31 '14 at 14:58
  • Why don't you just test it? For windows and fox osx it works just fine. – LHristov Jul 31 '14 at 15:03
  • Not really. Try using a filename longer than 260 characters, for example, using the long filename escaping prefix. Given that `Erase` is nothing more than a call to `DeleteFile`, why are you claiming that, somehow, `Erase` can to do `DeleteFile` cannot? `TFile.Delete` from `IOUtils` is the x-plat way. – David Heffernan Jul 31 '14 at 15:05