Hello acme_pjz,
acme_pjz {l Wrote}:Hi Edward_Lii,
Last time I have fixed some bugs under Windows in FileManager and commited it, but today after I update the working copy I found my changes are lost
Maybe you forget update before commiting changes?
I think I know what the problem is, I've changed some things back to original in the removeDirectory() functions.
And with the latest revision it's again not working under Windows.
The booleans are under Linux reversed since the return values of
unlink() and
rmdir() are zero for succes (bool r=false).
I think it would be best if we changed it that no matter what, true=succes and false=failure.
The first part of the code is ok.
bool removeDirectory(const char *path){
#ifdef WIN32
WIN32_FIND_DATAA f;
HANDLE h = FindFirstFileA((string(path)+"\\*").c_str(),&f);
#else
//Open the directory that needs to be removed.
DIR* d=opendir(path);
#endif
//Get the path length
size_t path_len = strlen(path);
//Boolean if the directory is emptied.
//True: succes False: failure
//Default is false because if the directory couldn't be opened we can return r(false).
bool r = false;
#ifdef WIN32
if(h!=NULL && h!=INVALID_HANDLE_VALUE) {
#else
//Check if the directory exists.
if(d) {
//Pointer to an entry of the directory.
struct dirent* p;
#endif
In the Linux code we enter a while loop as long as r!=true and there's an
diren(try) in the directory that needs to be removed.
#ifdef WIN32
do{
#else
//Loop the entries of the directory that needs to be removed.
while(!r && (p=readdir(d))) {
#endif
Here we create a second boolean r2 which contains true if something went wrong.
Which I changed to true if it succeeds and false if it fails.
//Boolean if the entry is deleted.
//True: succes False: failure
bool r2 = false;
char* buf;
size_t len;
/* Skip the names "." and ".." as we don't want to recurse on them. */
#ifdef WIN32
if (!strcmp(f.cFileName, ".") || !strcmp(f.cFileName, "..")) {
#else
if (!strcmp(p->d_name, ".") || !strcmp(p->d_name, "..")) {
#endif
//The filename is . or .. so we continue to the next entry.
continue;
} else {
#ifdef WIN32
//Get the length of the path + the directory entry name.
len = path_len + strlen(f.cFileName) + 2;
#else
//Get the length of the path + the directory entry name.
len = path_len + strlen(p->d_name) + 2;
#endif
buf = (char*) malloc(len);
Here's something wrong with the Windows code, in the else statement it should remove a file?
if(buf) {
#ifdef WIN32
_snprintf(buf, len, "%s\\%s", path, f.cFileName);
if(f.dwFileAttributes &; FILE_ATTRIBUTE_DIRECTORY){
r2 = removeDirectory(buf);
}else{
r2 = true; //Remove file here? <-----
}
#else
The Linux code shouldn't contain the unlink(buf)!=
0;.
This way
r2 will be true if the action failed.
r2 must be true when the action succeeded so, r2=unlink(buf)==0
struct stat statbuf;
snprintf(buf, len, "%s/%s", path, p->d_name);
if(!stat(buf, &statbuf)){
//Check if the entry is a directory or a file.
if (S_ISDIR(statbuf.st_mode)){
r2 = removeDirectory(buf);
}else{
r2 = unlink(buf)!=0;
}
}
#endif
//Free the buf.
free(buf);
}
There's no need to set r to r && r2, at least for the Linux code.
At the moment as long as r=false the while loop continues.
When a file fails to be deleted r2=true r will be set true, the while loop will be broken and the removeDirectory() function failed.
Even if we change r and r2 to true=success we don't need to edit this line, since the while loop will end depending on the status of the deletion of the diren(try). (
r2)
//We set r to r2 since r2 contains the status of the latest deletion.
// FIXME: should be r && r2 (??)
r = r2;
}
#ifdef WIN32
}while(FindNextFileA(h,&f));
FindClose(h);
#else
}
//Close the directory.
closedir(d);
#endif
}
The while loop can end in two ways, there are no directory entries any more(normal end).
Or a failure,
r=true.
That's why
!r is checked,
r is only true when it failed.
//The while loop has ended, meaning we (tried) cleared the directory.
//If r is true, meaning no errors we can delete the directory.
// FIXME: should be 'r', not '!r' (??)
if(/*!*/r){
// FIXME: the return value of rmdir is 0 means successful (??)
r = rmdir(path)==0;
}
This piece of code is wrong either way.
It returns false on success.
//Return the status.
return r;
}
The fixed code where true=success and false=failure is in the svn. (
here)
Sorry for the inconvenience.