Capturing files in a directory
Hi, I'm using the following code to capture all files in a directory -
but when I pass the "path" var such as "C:\SomeDir" ffd.cFileName is just simply "SomeDir" and is seen as a valid folder and no more files are processed.. I'm trying to get the contents in that folder. Where am I going wrong? I'm following this example here -
http://msdn.microsoft.com/en-us/libr...(v=vs.85).aspx
Code:
bool ListFiles(TCHAR* path) {
HANDLE hFind = INVALID_HANDLE_VALUE;
WIN32_FIND_DATA ffd;
TCHAR * spec = new TCHAR[_MAX_PATH];
static std::stack<TCHAR*> directories;
static std::vector<TCHAR *> files;
files.clear();
LPTSTR szANSIString[MAX_PATH] = {0};
hFind = FindFirstFile(path, &ffd);
if (hFind == INVALID_HANDLE_VALUE) {
return false;
}
do {
if (_tcscmp(ffd.cFileName, _T(".")) != 0 &&
_tcscmp(ffd.cFileName, _T("..")) != 0) {
if (ffd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) {
directories.push(ffd.cFileName);
}
else {
files.push_back(ffd.cFileName);
}
}
} while (FindNextFile(hFind, &ffd) != 0);
if (GetLastError() != ERROR_NO_MORE_FILES) {
FindClose(hFind);
return false;
}
FindClose(hFind);
hFind = INVALID_HANDLE_VALUE;
return true;
}
Re: Capturing files in a directory
You missed these lines from MSDN example:
Quote:
Code:
// Prepare string for use with FindFile functions. First, copy the
// string to a buffer, then append '\*' to the directory name.
StringCchCopy(szDir, MAX_PATH, argv[1]);
StringCchCat(szDir, MAX_PATH, TEXT("\\*"));
Re: Capturing files in a directory
Ah ok, is there any way to append to a TCHAR* without using a function?
Re: Capturing files in a directory
What is the problem with "using a function"?
Re: Capturing files in a directory
Theres no problem with that. i was just wondering why you cant append with a simple plus sign
Re: Capturing files in a directory
Quote:
Originally Posted by
drwbns
Theres no problem with that. i was just wondering why you cant append with a simple plus sign
Because TCHAR* is just a pointer. Adding some value to it means adding to the pointer!
If you want to use "a simple plus sign" to add some character text then you have to use some string class like std::string/wstring or MFC CString class.
Re: Capturing files in a directory
Ah ok i was just curious. thanks
Re: Capturing files in a directory
I tried using a reference instead of a static var for my files vector like so -
Code:
bool ListFiles(TCHAR* path, std::vector<TCHAR *> &files) {
but when I use it, I have a bunch of valid file names after the ListFiles call, but a few lines later, all the values are junk such as "iiiiiiiiiiiiiiiiiiiiiii", what could cause it?
Code:
if (ListFiles(dir, files)) {
for (std::vector<TCHAR *>::iterator it = files.begin();
it != files.end();
++it) {
TCHAR* text = new TCHAR[_MAX_PATH];
GetWindowText(textArea, text, _MAX_PATH);
StringCchCat(text, MAX_PATH, *it);
SetWindowText(textArea,text);
}
}
Re: Capturing files in a directory
Quote:
Originally Posted by
drwbns
I tried using a reference instead of a static var for my files vector like so -
Code:
bool ListFiles(TCHAR* path, std::vector<TCHAR *> &files) {
but when I use it, I have a bunch of valid file names after the ListFiles call, but a few lines later, all the values are junk such as "iiiiiiiiiiiiiiiiiiiiiii", what could cause it?
First, what is "textArea"?
Second, you changed "ListFiles". Don't you think it makes a difference we see what this new ListFiles() consists of.
Third, your code has a lot wrong with it.
1) It has memory leaks. You call "new[]" and nowhere do you call "delete[]" to release the memory.
2) Why did you even introduce "new[]" in the first place, while you're using vector? A vector is supposed to relieve you from using new[]/delete[] since it is a dynamic array container, but you're not using vector for the purpose for which it was intended for. If you're using std::vector, but at the same time using new[]/delete[], then it's time to take a step back and look carefully at what you're doing, since somewhere you're making a design mistake, misstep, or oversight (unless you're a C++ pro and you have a good reason to be using both).
Things like this:
Code:
TCHAR* text = new TCHAR[_MAX_PATH];
are totally unnecessary. Not only that, you're doing this in a loop and creating huge memory leaks without a call to delete[].
Moreover, why even use dynamically allocated memory anyway, when you know the size of the array? It isn't even dynamic. So you're needlessly slowing your code down by calling the allocator and you're creating a memory leak with the code you have now.
Code:
TCHAR text[_MAX_PATH];
All you need is a TCHAR array that has _MAX_PATH elements. You make this mistake in this new code and the original code you have at the top of the thread.
If _MAX_PATH were some huge number into the millions, then maybe that would be reason to allocate dynamically. But since _MAX_PATH is something very small, just create a normal, non-dynamic array of _MAX_PATH size. Even if you need to allocate dynamically, std::vector<> should have been used instead of new[].
Since it looks like you're using Visual C++, you could have just used CString (if VC++ is higher than 6.0, since CString is not tied to MFC in higher versions of VC++), or a level down from that, std::string/std::wstring and choose the right one depending on build type (Unicode/ANSI). Then replace all of those TCHAR* with CString or std::(w)string.
The bottom line being that you shouldn't need to write code that ties you up in knots with respect to memory allocation. The code should be immune to memory leaks even if an exception is thrown, and using things such as CString/std::(wstring) ensures this due to the destructors of these classes cleaning up themselves automatically.
Code:
#include <vector>
#include <string>
// use the right string class for the build type
#ifndef UNICODE
typedef std::string StringClass;
#else
typedef std::wstring StringClass;
#endif
typedef std::stack<StringClass> StringStack;
typedef std::vector<StringClass> StringVector;
//...
bool ListFiles(const StringClass& path, StringVector &files)
{
HANDLE hFind = INVALID_HANDLE_VALUE;
WIN32_FIND_DATA ffd;
static StringStack directories;
LPTSTR szANSIString[MAX_PATH] = {0};
hFind = FindFirstFile(path, &ffd);
if (hFind == INVALID_HANDLE_VALUE)
return false;
do {
if (_tcscmp(ffd.cFileName, _T(".")) != 0 &&
_tcscmp(ffd.cFileName, _T("..")) != 0)
{
if (ffd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)
{
directories.push(ffd.cFileName);
}
else {
files.push_back(ffd.cFileName);
}
}
} while (FindNextFile(hFind, &ffd) != 0);
if (GetLastError() != ERROR_NO_MORE_FILES) {
FindClose(hFind);
return false;
}
FindClose(hFind);
hFind = INVALID_HANDLE_VALUE;
return true;
}
//...
for (StringVector::iterator it = files.begin();
it != files.end();
++it)
{
TCHAR text[_MAX_PATH];
GetWindowText(textArea, text, _MAX_PATH);
StringCchCat(text, MAX_PATH, *it);
SetWindowText(textArea,text);
}
}
Also, hopefully you know that your first ListFiles functions is not thread-safe due to the static types used in the function.
Regards,
Paul McKenzie
Re: Capturing files in a directory
TextArea is a textBox
Code:
bool ListFiles(TCHAR* path, std::vector<TCHAR *> &files, std::vector<TCHAR*> &directories) {
HANDLE hFind = INVALID_HANDLE_VALUE;
WIN32_FIND_DATA ffd;
TCHAR spec[_MAX_PATH];
directories.clear();
files.clear();
LPTSTR szANSIString[MAX_PATH] = {0};
StringCchCat(path, MAX_PATH, TEXT("\\*"));
hFind = FindFirstFile(path, &ffd);
if (hFind == INVALID_HANDLE_VALUE) {
return false;
}
do {
if (_tcscmp(ffd.cFileName, _T(".")) != 0 &&
_tcscmp(ffd.cFileName, _T("..")) != 0) {
if (ffd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) {
directories.push_back(ffd.cFileName);
}
else {
files.push_back(ffd.cFileName);
}
}
} while (FindNextFile(hFind, &ffd) != 0);
if (GetLastError() != ERROR_NO_MORE_FILES) {
FindClose(hFind);
return false;
}
FindClose(hFind);
hFind = INVALID_HANDLE_VALUE;
return true;
}
Re: Capturing files in a directory
Quote:
Originally Posted by
drwbns
TextArea is a textBox
Did you mean it is a window handle of "a textBox"?
Re: Capturing files in a directory
yes, sorry. I'm just trying to understand TCHAR and it's correct usage for the most part. Is it possible to make a vector containing a TCHAR[] array or is that just a no no? I tried Paul's StringClass suggestion, but how do I convert a StringVector to a LPSTR when needed?
Re: Capturing files in a directory
Quote:
Originally Posted by
drwbns
yes, sorry. I'm just trying to understand TCHAR and it's correct usage for the most part. Is it possible to make a vector containing a TCHAR[] array
Where in your code do you need an LPSTR? All I see in your code is to gather the filenames.
Secondly, StringVector is a vector of CString/std::(wstring). It isn't just one string, so there is no such thing as converting a StringVector to an LPSTR. What is possible is to take one of the stirngs in the StringVector and return an LPCTSTR -- that's why I asked why you need LPSTR instead of LPCTSTR.
Regards,
Paul McKenzie
Re: Capturing files in a directory
dir is a StringVector like you suggested - but I'm getting a crash after calling buttonPush -
Code:
bool buttonPush(HWND hWnd, StringVector &dir ) {
BROWSEINFO bi;
TCHAR szDir[MAX_PATH];
LPITEMIDLIST pidl;
LPMALLOC pMalloc;
if (SUCCEEDED(SHGetMalloc(&pMalloc)))
{
ZeroMemory(&bi,sizeof(bi));
bi.hwndOwner = hWnd;
bi.pszDisplayName = 0;
bi.pidlRoot = 0;
bi.ulFlags = BIF_RETURNONLYFSDIRS | BIF_STATUSTEXT | BIF_USENEWUI | BIF_NONEWFOLDERBUTTON;
bi.pszDisplayName = szDir; // Address of a buffer to receive the display name of the folder selected by the user
bi.lpszTitle = _T("Select a folder"); // Title of the dialog
bi.lpfn = BrowseCallbackProc;
pidl = SHBrowseForFolder(&bi);
if (pidl)
{
BOOL bRet = ::SHGetPathFromIDList(pidl, const_cast<char *>(dir[0].c_str())); // dir gets a bad ptr here, how can I do this with a StringVector?
pMalloc->Free(pidl);
pMalloc->Release();
if(bRet) return true;
}
}
return false;
}
Code:
SetWindowText(textBox2, const_cast<char *>(dir[0].c_str()));
Is const casting safe to do in this scenario?
Re: Capturing files in a directory
Quote:
Originally Posted by
drwbns
dir is a StringVector like you suggested - but I'm getting a crash after calling buttonPush -
1) You don't check to see if "dir" has any strings (is there really a value at dir[0])? You cannot access dir[0] if dir is an empty vector of strings.
2) You should not cast away the constness of the return of c_str(). It is const for a reason -- attempting to write to it is undefined behaviour.
Why not just declare a simple array of TCHAR, call the SHGetPathFromIDList function with this array, and then just assign dir[0] with the array?
Code:
TCHAR SomeArray[_MAX_PATH];
BOOL bRet = ::SHGetPathFromIDList(pidl, SomeArray);
dir[0] = SomeArray; // assuming that SomeArray is NULL terminated
Regards,
Paul McKenzie
Re: Capturing files in a directory
Ok thanks, is this ok to do? Also, why is it that I'm only calling dir.push_back once, yet I have 2 elements? The first is empty, the 2nd is my push_back call.
Code:
TCHAR SomeArray[_MAX_PATH];
buttonPush(hWnd,dir);
_tcscpy(SomeArray, dir[1].c_str());
SetWindowText(textBox2, SomeArray);
Code:
bool buttonPush(HWND hWnd, StringVector &dir ) {
BROWSEINFO bi;
TCHAR szDir[MAX_PATH];
LPITEMIDLIST pidl;
LPMALLOC pMalloc;
if (SUCCEEDED(SHGetMalloc(&pMalloc)))
{
ZeroMemory(&bi,sizeof(bi));
bi.hwndOwner = hWnd;
bi.pszDisplayName = 0;
bi.pidlRoot = 0;
bi.ulFlags = BIF_RETURNONLYFSDIRS | BIF_STATUSTEXT | BIF_USENEWUI | BIF_NONEWFOLDERBUTTON;
bi.pszDisplayName = szDir; // Address of a buffer to receive the display name of the folder selected by the user
bi.lpszTitle = _T("Select a folder"); // Title of the dialog
bi.lpfn = BrowseCallbackProc;
pidl = SHBrowseForFolder(&bi);
if (pidl)
{
TCHAR SomeArray[_MAX_PATH];
BOOL bRet = ::SHGetPathFromIDList(pidl, SomeArray);
dir.push_back(SomeArray);
pMalloc->Free(pidl);
pMalloc->Release();
if(bRet) return true;
}
}
return false;
}
Re: Capturing files in a directory
Quote:
Originally Posted by
drwbns
why is it that I'm only calling dir.push_back once, yet I have 2 elements?
What we see in code statically and what the actual flow of the code happens to be are two different things. You have 2 elements because you have 2 elements. That is what you did, and that is what was done. I don't know what else to tell you. Maybe you should debug your code to figure this out. All I see is that "dir" just pops out of nowhere into your code, without any indication where it was declared, where it came from, what accessed it previously, etc.
Sure I see only one call to push_back, but how do I know if it is done once, twice, or twenty times when the program is run? You don't have a full program posted, so no one knows, how, when, or where these functions are called, or with what data they are being called with.
Also,
Code:
_tcscpy(SomeArray, dir[1].c_str());
How do you know there is an element at dir[1]? Accessing non-existent elements in an array or vector using [] leads to undefined behaviour.
Regards,
Paul McKenzie
Re: Capturing files in a directory
This goes back to your other discussion thread 'TCHAR correct usage'. Why are you using StringVector as a function argument when you are only returning a single name? As per the other thread, why not use string& dir as a function parameter?
Re: Capturing files in a directory
Ah ok, I changed it to a StringClass
Re: Capturing files in a directory
A word to the wise. Do you know what your program is trying to do before you start to code it? is this documented? Have you designed it before starting to code? Have you defined AND DOCUMENTED what functions are required and their interfaces? Programming is a lot more than just coding. In your code might I suggest that you insert comments before each function describing what it is supposed to do, what it returns and the purpose of each of its parameters, whether they are [in] only, [out] only or both [in/out], any assumptions made about the value(s) of the [in] parameters, the state of the [out] parameters if an error occurs etc etc etc. Time spent on documentation is time well spent. Others trying to understand your code have an idea of what it should be doing and when you come back to your code in a week/month/year's time and you're forgotten what you did and why then the comments will help - also helps when trying to use a function. The interfaces to a function should be the simplest that is needed to full fill its purpose which should be only one thing.
Re: Capturing files in a directory
You might find it helpful to read 'Problem Solving, Abstraction & Design Using c++' by Friedman and Koffman. Especially sections 1.5, 1.6 and chapter 6 regarding program design. If you can't find a copy in a library, Amazon UK have the 2000 version used for £1.96 (Plus p&p).
Re: Capturing files in a directory
Is there a way to get a const char * from a std::string::iterator when iterating over a std::string?
