Win32API DeleteFile and some other problems

Started by
7 comments, last by Eriond 19 years, 7 months ago
I'm trying to make a program that deletes duplicates of files with the same name (ignoring extension) but I have some problems. Here is all code, the problems are at the end (atleast I think so :P).
#include <windows.h>
#include <string>
#include <vector>
#include <fstream>
#include <algorithm>
using namespace std;

typedef vector< WIN32_FIND_DATA > FileList;

void Split( vector< string >& out,
			const string& str,
			const string& delim )
{
	string::size_type firstPos = 0;
	string::size_type secondPos = str.find_first_of( delim );
	out.push_back( str.substr( firstPos, secondPos ) );
	while( secondPos != string::npos )
	{
		firstPos = secondPos + 1;
		secondPos = str.find_first_of( delim, firstPos );
		out.push_back( str.substr( firstPos, secondPos - firstPos ) );
	}
}

bool IsDirectory( const WIN32_FIND_DATA& data )
{
	return data.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY;
}

bool NoExtCmp( const WIN32_FIND_DATA& data1, const WIN32_FIND_DATA& data2 )
{
	string s1 = data1.cFileName, s2 = data2.cFileName;
	s1 = s1.substr( 0, s1.size() - s1.find_last_of( "." ) );
	s2 = s2.substr( 0, s2.size() - s2.find_last_of( "." ) );
	return s1 == s2;
}

int WINAPI WinMain( HINSTANCE instance, HINSTANCE prevInstance, 
					char* cmdLine, int cmdShow )
{
	FileList files;
	ofstream file( "log.txt" );
	vector< string > cmd;
	Split( cmd, cmdLine, " " );
	string dir;
	try
	{
		dir = cmd.at( 0 );
	}
	catch( out_of_range& e )
	{
		file << "first commandline argument should be the path to search" << endl;
		return -1;
	}

	file << "Target directory: " << dir << '\n' << endl;
	dir += "\\*";

	WIN32_FIND_DATA FindFileData;
	HANDLE hFind = FindFirstFile( dir.c_str(), &FindFileData );

	if( hFind == INVALID_HANDLE_VALUE ) 
	{
		file << "Invalid file handle. Error is " << GetLastError() << endl;
		return -1;
	}
	else
	{
		if( !IsDirectory( FindFileData ) )
		{
			files.push_back( FindFileData );
		}
		while( FindNextFile( hFind, &FindFileData ) )
		{
			if( !IsDirectory( FindFileData ) )
			{
				files.push_back( FindFileData );
			}
		}

		DWORD dwError = GetLastError();
		if( dwError == ERROR_NO_MORE_FILES )
		{
			FindClose( hFind );
		}
		else
		{
			file << "FindNextFile error. Error is " << dwError << endl;
			return -1;
		}
	}

	file << "Before:" << endl;
	for( FileList::iterator it( files.begin() ), end( files.end() );
		 it != end; ++it )
	{
		file << it->cFileName << '\n';
	}
	file << endl;

	FileList::iterator pos = unique( files.begin(), files.end(), NoExtCmp );

	for( FileList::iterator it( pos ), end( files.end() ); it != end; ++it )
	{
		if( DeleteFile( it->cFileName ) )
		{
			file << it->cFileName << " deleted\n";
		}
		else
		{
			file << "could not delete " << it->cFileName << '\n';
		}
	}
	file << endl;
	files.erase( pos, files.end() );

	file << "After:" << endl;
	for( FileList::iterator it( files.begin() ), end( files.end() );
		 it != end; ++it )
	{
		file << it->cFileName << '\n';
	}
	return 0;
}

After I run this log.txt looks like this:
Target directory: c:\test

Before:
hehe.tr1
hehe.tr2
okej.tr1
test.tr1
test.tr2
test.tr3
test.tr4
test.tr5
test.tr6

could not delete test.tr1
could not delete test.tr2
could not delete test.tr3
could not delete test.tr4
could not delete test.tr5
could not delete test.tr6

After:
hehe.tr1
okej.tr1
test.tr1
The first question I have is, why couldn't the files be deleted? The second, the files that should be deleted are test.tr2-6 and hehe.tr2, but the program tries to delete all files named test. When I erase the duplicates from the vector the right files are left in it. I don't understand why that is happening. Probably some stupid mistake after the call to uniqe.
Advertisement
After any windows API function fails, they generally set the code returned by GetLastError, so you can see specifically what went wrong.

You can use the following code to display the exact error message:
void DisplayWinAPIError(void){	LPVOID lpMsgBuf;	FormatMessage(FORMAT_MESSAGE_ALLOCATE_BUFFER|FORMAT_MESSAGE_FROM_SYSTEM		|FORMAT_MESSAGE_IGNORE_INSERTS,	    NULL,GetLastError(),MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT),	   (LPTSTR) &lpMsgBuf, 0, NULL);	MessageBox(NULL,(LPCTSTR)lpMsgBuf, "Error", MB_OK | MB_ICONINFORMATION);	LocalFree(lpMsgBuf);}
"Walk not the trodden path, for it has borne it's burden." -John, Flying Monk
I'm guessing that DeleteFile() is expecting an absolute path. You're providing just a local file name, so you're likely going to need to prepend the full folder path of the file. A quick way would be:

if( DeleteFile( ( dir + "\\" + it->cFileName ).c_str() ) )


Also, although I haven't used std::unique() before, after looking at the docs, it appears that the returned iterator is the end of the sequence, so your initial value for it should be files.begin(), and your terminating value should be pos. Try that out and see what happens.

If you want to really find out what std::unique does, copy that loop that displays the contents of the vector right before the call to std::unique, and also display the contents of the vector immediately after the call to std::unique, using files.begin() and files.end(), ignoring the returned value from std::unique.
"We should have a great fewer disputes in the world if words were taken for what they are, the signs of our ideas only, and not for things themselves." - John Locke
I have tested to erase( iterator from uniqe, end ); and it worked as it should. The last loop outputs what it should too, but the loop which tries to remove files, try to remove the wrong files.

Edit: Now the files were deleted, but not the right files. The output:
Target directory: c:\testBefore:hehe.tr1hehe.tr2okej.tr1test.tr1test.tr2test.tr3test.tr4test.tr5test.tr6test.tr1 deletedtest.tr2 deletedtest.tr3 deletedtest.tr4 deletedtest.tr5 deletedtest.tr6 deletedAfter:hehe.tr1okej.tr1test.tr1
test.tr1 should not be deleted but hehe.tr2 should. The files left should be those under After.
Doh! Yeah, while trying to sort through your code and find any errors, I ended up forgetting the actual goal you were trying to achieve: delete everything not from files.begin() to pos.

After looking at the docs some more, it looks like std::unique() doesn't try at all to preserve all the non-unique items. All it cares about is getting the list to be unique from .begin() to its return value. You're going to either have to write your own function to move all the unique items to the beginning, and all the duplicates to the end (the step that std::unique doesn't do), or you're going to have to maintain two lists: the first list is the list of all files, and the second list is the list of just the unique files. Then, go through all the files, and if the file isn't in the unique list, delete it.
"We should have a great fewer disputes in the world if words were taken for what they are, the signs of our ideas only, and not for things themselves." - John Locke
The thing I find strange is that the right things are removed with erase. AFAIK I use the same range for erase as for DeleteFile :/
You might be able to use std::unique(), and then std::set_difference() between the unique list and the original list to get a list of the non-unique files. If it does what I think it should, then it'll get the job done, although by this point, the program is probably getting obscenely inefficient. But that might not matter much.
"We should have a great fewer disputes in the world if words were taken for what they are, the signs of our ideas only, and not for things themselves." - John Locke
I think after calling std::unique(), your vector contains the following:
hehe.tr1  //Copied from index 1 to index 1 (overwrote "hehe.tr1")okej.tr1  //Copied from index 3 to index 2 (overwrote "hehe.tr2")test.tr1  //Copied from index 4 to index 3 (overwrote "okej.tr1")test.tr1  //returned iterator position, everything after this pointtest.tr2  //  hasn't been touched.test.tr3test.tr4test.tr5test.tr6
"We should have a great fewer disputes in the world if words were taken for what they are, the signs of our ideas only, and not for things themselves." - John Locke
I think I'll go to sleep and try to fix it tomorrow. Thank you for your help. I'll probably post more tomorrow, because it wouldn't surprise me if I run into more problems :)

This topic is closed to new replies.

Advertisement