Review my file-searching program

It's my first window program. It simply searches for a specified file in the whole computer.

File Search.h (header file that contains the prototypes of fileSearcher class methods.)

#ifndef UNICODE
#define UNICODE
#endif

#include 
#include 

namespace fileSearch
{
class fileSearcher
{
public: 
    fileSearcher();
    ~fileSearcher(); //So far, this class doesn't allocate memory on the heap,therefore destructor is empty 
    void getAllPaths(const TCHAR* fileName,std::queue &output); 
    /*Returns all matching pathes at the current local system. Format:
    [A-Z]:\FirstPath\dir1...\fileName
    [A-Z]:\SecondPath\dir2...\fileName
    ...
    [A-Z]:\NPath\dirN...\fileName
    */

    void findFilesRecursivelly(const TCHAR* curDir,const TCHAR* fileName,std::queue &output);
    //Searches for the file in the current and in sub-directories. Sets nothingFound=false if the file has been found.

private:    
    static const  int MAX_LOCATIONS = 20000;

    bool nothingFound;
    void endWithBackslash(TCHAR* string);
};
}

File Search.cpp ( ... definitions)

```
#ifndef UNICODE
#define UNICODE
#endif

#include "File Search.h"

using namespace fileSearch;

fileSearcher::fileSearcher()
{
nothingFound = true;
}

fileSearcher::~fileSearcher()
{

}

void fileSearcher::getAllPaths(const TCHAR* fileName,std::queue &output)
{
TCHAR localDrives[50];
TCHAR currentDrive;
int voluminesChecked=0;
TCHAR searchedVolumine[5];

nothingFound = true;

if(!wcslen(fileName))
{
output.push(TEXT("Invalid search key"));
return;
}
GetLogicalDriveStrings(sizeof(localDrives)/sizeof(TCHAR),localDrives);

//For all drives:
for(int i=0; i = 65 && localDrives[i] &output)
{
HANDLE hFoundFile;
WIN32_FIND_DATA foundFileData;
TCHAR* buffer;

buffer = new TCHAR[MAX_PATH+_MAX_FNAME];

wcscpy(buffer,curDir);
endWithBackslash(buffer);
if(!SetCurrentDirectory(buffer)) return;

//Fetch inside curren

A few points:

-
Best Practice: There's no need to define UNICODE in File Search.h since it's defined at the top of those files that include it. Consider creating a configuration header that gets included by the .cpp first and let it handle the defines and other preprocessor logic. That way if you decide to make a non-UNICODE build, you only need to change one definition, not two or three.

-
Best Practice: Class names in C++ generally follow one of two conventions: CamelCaseStyle or underscore_style. Consider using one of these for your class.

-
Potential Build Error / Best Practice: Good use of TCHAR and the TEXT macro to conform to the Windows function definitions. Be sure to use the macro instead of directly using L"..." and L'.', also, so that the code can compile a non-UNICODE build. Same goes for the string-manipulating functions: wcscpy (should be _tcscpy), wcscat (should be _tcscat), and wcscmp (should be _tcscmp). More on strings later, though, since all of this (including the use of TCHAR*) is the hard way to manage strings in C++.

-
Memory Leak: In fileSearcher::fineFilesRecursivelly, buffer is allocated with new, but never deleted. Currently the function can return in two places, so you'll need to have delete [] buffer before those two places (more on strings like buffer next). [edit: buffer is allocated twice in the function. Be sure that it's freed before allocating it again.]

-
Best Practice: You're using C-style strings in your C++ code, which means you need to allocate them yourself (like buffer), deallocate them yourself, and call the various string-managing functions. Instead, consider using C++'s std::basic_string-derived classes to handle the tedious details. I think the related changes you'd need to make are out of scope for a code review because of the heavy use of C-style strings in your code, so check out Stack Overflow's many questions and answers on the topic.

-
Potential Runtime Error / Best Practice: You're using std::queue to make a queue of strings, but the strings are not being copied to the queue, only referenced. The string's memory is not guaranteed to be correct once the string variable goes out of scope (I'm a little surprised it works [edit: it's not as broken as I first thought, but I still recommend the change mentioned here] ). You'll want to use std::queue> to ensure that copies are created and destroyed. More on strings in the previous note.

Shortcomings that you mentioned:

-
The leg-work in your code is being done from WindowProc, so additional messages cannot be processed until the work is finished. You could use a separate thread to do the work, but that topic is beyond the scope of this review.

-
I think the memory leak is buffer, as mentioned above.

StackExchange Code Review Q#16303, answer score: 4