Solved

a findfiles question

Posted on 2001-07-25
8
395 Views
Last Modified: 2010-04-02
i wrote a piece of code that searches for a file.  I would like someone to look through this code and see if they can see anything wrong with it.  could someone improve it for me?  What its suppose to do is search through a given directory and everything below it for a file. it really doesn't work that way right now.  so it needs improvement.

WIN32_FIND_DATA fileFindData;

CString FindFiles(CString strStartDir, CString strFileName)
{
     HANDLE hFile = FindFirstFile(strStartDir + "\\" + strFileName, &fileFindData);
     CString csFileName(fileFindData.cFileName);
     CString csSubDir(""), strFound("ERROR - file not found");
         
     if (hFile != INVALID_HANDLE_VALUE)
     {
          do
          {
               csFileName = fileFindData.cFileName;
               if ((fileFindData.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) && (csFileName != ".") && (csFileName != ".."))
               {
                    csSubDir = strStartDir + "\\" + csFileName;
                    FindFiles(csSubDir);
               }

               if (csFileName.Find(".txt") != -1)
                    strFound = strStartDir + "\\" + fileFindData.cFileName;
          } while (FindNextFile(hFile, &fileFindData) && strFound == "ERROR - file not found");
          FindClose(hFile);    
     }
     return strFound;
}
0
Comment
Question by:simongod
  • 4
  • 3
8 Comments
 
LVL 22

Expert Comment

by:nietod
Comment Utility
The basic idea looks fine.  

One problem I see is that you are not filtering out the two "special" directories "." and "..".   Every directory has an entry called "." that is a directory.  this entry ferers to the directory itself!.  So if you come to direcotry  "." and then seearch "." you will be searching the directory you were already searching.  And that dearch will find "." and search the directory again.  and again and again.  You will recurse forever.

Similarly all directories except for the root directory has an entry called  ".." which is for the parent directory.  if you search a directory and encouter ".." you will then search the parent directory, which you were searching earlier.  That will then lead to this directory, and then back to the parent,  then to this director and so on forever.

So you need to skipp these two directories.

Other than that it looks fine.  What is the problem you are getting?  maybe that was the problem?
0
 
LVL 22

Expert Comment

by:nietod
Comment Utility
opps,   No you have that in there don't you.  I missed that.


how can this be right

>>                  FindFiles(csSubDir);

The FindFiles function takes 5wo paremeters, you are passing only one.  that shouldn't even compile.

That should be

                  FindFiles(csSubDir,strFileName);

Also the loop condition

>>        } while (FindNextFile(hFile, &fileFindData) && strFound == "ERROR - file not found");

is very strange.  Using a string to control a loop like that is very unuusual and pretty innefficient.  Why are you doing that?

0
 
LVL 2

Author Comment

by:simongod
Comment Utility
sorry it seems that i forgot to mention a few things.

the findfiles can accept 0,1or 2 arguments.  I have the function declared with the two arguments equaling NULL.  Does this help?

as for the string controling the loop.  The default for the strFound is "ERROR - file not found".  what the loop is looking for is for that to change.  if it changes then the file and its directory were found.

does that help?


The problem that i am having is when the file is found it jumps out of that while loop and returns the file name (return strFound;).  After it has completed the the next line of execution is the FindFiles(csSubDir) that is located in the if.  That seems strange and causes and assertion
0
 
LVL 2

Expert Comment

by:The Master
Comment Utility
The problem looks to be in your while statement.  It's going to analyze both conditions in order before evaluating the entire while().  In other words, it's going to call FindNextFile() before it evaluates strFound.  Change your while() statement to...

} while (strFound == "ERROR - file not found" && FindNextFile(hFile, &fileFindData));

... and that should do the trick.

Hope this helps!
0
Enabling OSINT in Activity Based Intelligence

Activity based intelligence (ABI) requires access to all available sources of data. Recorded Future allows analysts to observe structured data on the open, deep, and dark web.

 
LVL 2

Expert Comment

by:The Master
Comment Utility
Oops - nevermind.  I thought you had named your func FindNextFile as well.  My bad.
0
 
LVL 22

Expert Comment

by:nietod
Comment Utility
>> he findfiles can accept 0,1or 2 arguments.  I have the function
>> declared with the two arguments equaling
>> NULL.  Does this help?
Not at all.

I don;t see how you could expect it t work when an argument is not specified.  i..e I don't know what you want to have happen AND  I don't think I see any code that looks for this case and will make something happen in this case.

I woudl expect to see code like

if (strStartDir.length() == 0); // If no start directory, then
   strStartDir = "\\"; // use the root directory.

if (strFileName.length() == 0); // If no file name, then
   strFileName = "*.*"; // Look for any file.


Now that I think about it, the code

   FindFirstFile(strStartDir + "\\" + strFileName, &fileFindData);

is probably wrong too.  This will search that directory for any file name matching strFileName's contents.  That is fine since that is ultimately what you want. BUT it will also return only directories that match that as well.  You want to find ALL directory names and any files names that match the search skeleton.   There is no way to do that using FindFirstFile, so you have to do it manually.  You need to search the directory for *.*.  This will get your all direcotries--whic is what you want.  It also gets you all files, you will need to test each file name to see if it matches the specified search string.

OR

use two passes per directory.  First search the directory fir any files that match the search string.  Then if there is none found, search the drectory in a 2nd loop for *.* and look for entries that are sub-directories.  Then search these.    

This 2nd method is the one I usually use.

>>  if it changes then the file and its directory were found.
Why not use a boolean varaible to indicate when a match is found.  Its likely to be 100s or even 1000s of times faster.

Finally I don't see how its supposed to return information to the caller.  What is it supposed to return?  When you mke a recusrive call you ignore the return value. That is probalby wrong.

It might help if you explained what it is supposed to do.  In detail.   its very hard to understand what the goal is.
0
 
LVL 22

Expert Comment

by:nietod
Comment Utility
>> The problem looks to be in your while statement
It looks to me like there are little problems everywhere.  Its unclear what is a problem and what isn;t since its not clearwhat its supposed to do, but lots of things don't add up that is for sure.
0
 
LVL 2

Accepted Solution

by:
The Master earned 100 total points
Comment Utility
Ok, let's take another stab at this.  I don't understand your last comment - that you have the function declared with the arguments equalling NULL.  Are these CString pointers you're passing in?

However, if the FindFiles(csSubDir) actually recurses into the same function, then it looks like a problem.  Let's say you have the following directories on your hard drive:

C:\test
C:\test\subdir

and you are looking in "test" for a file with the extension ".txt", but the only text file exists in "test\subdir".  So your code will recurse into FindFiles again (via the FindFiles(csSubDir) in the if block), find the text file, and return.  But it's returning to the original FindFiles(), just after FindFiles(csSubDir).  Your strFound that is local to that function is still going to equal "ERROR - file not found".

Try changing that line in the if block to:

strFound = FindFiles(csSubDir);

Good luck!
0

Featured Post

Better Security Awareness With Threat Intelligence

See how one of the leading financial services organizations uses Recorded Future as part of a holistic threat intelligence program to promote security awareness and proactively and efficiently identify threats.

Join & Write a Comment

Errors will happen. It is a fact of life for the programmer. How and when errors are detected have a great impact on quality and cost of a product. It is better to detect errors at compile time, when possible and practical. Errors that make their wa…
Introduction This article is the first in a series of articles about the C/C++ Visual Studio Express debugger.  It provides a quick start guide in using the debugger. Part 2 focuses on additional topics in breakpoints.  Lastly, Part 3 focuses on th…
The viewer will learn how to pass data into a function in C++. This is one step further in using functions. Instead of only printing text onto the console, the function will be able to perform calculations with argumentents given by the user.
The viewer will be introduced to the technique of using vectors in C++. The video will cover how to define a vector, store values in the vector and retrieve data from the values stored in the vector.

772 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

10 Experts available now in Live!

Get 1:1 Help Now