Thread: directory searching causes a windows error

  1. #1
    Registered User
    Join Date
    Mar 2007
    Posts
    416

    directory searching causes a windows error

    This is basically a continuation of my old thread about file searching, but since I was completely and utterly wrong on the thought behind it I made a new thread. Old thread is here.

    I looked over Elysia's example of looking through the files and every time I get a better understanding of it. I basically copied the example in my own terms in to my program, and now it comes up with a windows error (the "Send Error Report", "Don't Send" dialog box). I've tried eliminating things one at a time to see what the problem is but nothing points in a new direction. Where is my code coming up with this error?

    Here is Elysia's...
    Code:
    bool FindFilesInternal(pp<FindFilesInternalStruct> pArgs, pp< vector<CString> > s_vFiles, UINT64& rCurrentCount, UINT64& rTotalCount)
    {
    	WIN32_FIND_DATA fDataDirs, fDataFiles;
    	HANDLE hFileDirs = NULL, hFileFiles = NULL;
    
    	if (pArgs->strFolder.Right(1) == _T("\\")) pArgs->strFolder.Delete(pArgs->strFolder.GetLength());
    
    	hFileDirs = FindFirstFile(pArgs->strFolder + _T("\\*.*"), &fDataDirs);
    	if (hFileDirs == (HANDLE)-1) return false; // Apparently we couldn't search this directory...
    	hFileFiles = FindFirstFile(pArgs->strFolder + _T("\\") + pArgs->strPattern, &fDataFiles);
    	
    	if (hFileFiles != (HANDLE)-1)
    	{
    		while (FindNextFile(hFileFiles, &fDataFiles) && GetLastError() == ERROR_NO_MORE_FILES) // Search for files in local directory
    		{
    			if (pArgs->pCancel && *pArgs->pCancel) return true;
    			if ( !(fDataFiles.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) )
    			{
    				rCurrentCount++;
    				rTotalCount++;
    				if (*pArgs->p_bRequest) // Due to thread safety, data has to be requested. Local copies of everything is kept. This avoids expensive locking.
    				{
    					if (pArgs->p_nCurrentFile) *pArgs->p_nCurrentFile = rCurrentCount;
    					if (pArgs->p_strCurrentFile) *pArgs->p_strCurrentFile = pArgs->strFolder + _T("\\") + fDataFiles.cFileName;
    					if (pArgs->pTotalFiles) *pArgs->pTotalFiles = rTotalCount;
    					*pArgs->p_bRequest = false;
    					SetEvent(pArgs->hRequestDataEvent);
    				}
    				if (!pArgs->bCount)
    					s_vFiles->push_back(pArgs->strFolder + _T("\\") + (CString)fDataFiles.cFileName);
    			}
    		}
    	}
    
    	if (pArgs->bSearchSubFolders)
    	{
    		while (FindNextFile(hFileDirs, &fDataDirs) && GetLastError() == ERROR_NO_MORE_FILES) // Search for directories
    		{
    			if (pArgs->pCancel && *pArgs->pCancel) return true;
    			if ((fDataDirs.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) && _tcscmp(fDataDirs.cFileName, _T(".")) != 0 && _tcscmp(fDataDirs.cFileName, _T("..")) != 0)
    			{
    				ppnew<FindFilesInternalStruct> pArgs2;
    				*pArgs2 = *pArgs;
    				pArgs2->strFolder = pArgs->strFolder + _T("\\") + fDataDirs.cFileName;
    				FindFilesInternal(pArgs2, s_vFiles, rCurrentCount, rTotalCount);
    			}
    		}
    	}
    
    	return true;
    }
    Here is mine... It might look choppy cause I am still learning.

    Code:
    bool Search(SEARCHFILES pArgs, int &fCount, int &dCount, int &total){
        
        LPSTR lBuffer; //storage for _getdcwd
        LPSTR buffer; //displayed storage for _getdcwd
        LPSTR pbuffer; //storage for parent
        LPSTR fBuffer; //storage for file name to be checked
        
        WIN32_FIND_DATA Files,Dirs; //structure for handling all the files
        HANDLE hFiles,hDirs; //handles for FindNextFile()
        
        strcat(fBuffer,pArgs.StartFolder); //set up new folder to search in
        strcat(fBuffer,"\\*.*"); //find all files in new folder
        
        hFiles = FindFirstFile(fBuffer,&Files); //find the first, associated with files
        hDirs = FindFirstFile(fBuffer,&Dirs); //find the first, associated with directories
        
        if(hFiles == INVALID_HANDLE_VALUE) return false; //if the file handle is invalid, return false
        else if(hFiles != INVALID_HANDLE_VALUE)
        {
            while(FindNextFile(hFiles,&Files) && GetLastError() != ERROR_NO_MORE_FILES)
            {
                if(Files.dwFileAttributes != FILE_ATTRIBUTE_DIRECTORY)
                {
                    fCount++; //increase file count
                    //SendMessage(hStatus,SB_SETTEXT,1,(LPARAM)"files searched: &#37;d fCount"); //this might be wrong
                    //but it's not the cause of the problem (editted out)
                    
                    fBuffer = _getcwd(lBuffer,MAX_PATH); //get current directory
                    strcat(fBuffer,Files.cFileName); //get full path for file (NOT USED YET)
                    SetDlgItemText(Hwnd,TEST,Files.cFileName); //show the current file
                }
            }
        }
        else{
            SetDlgItemText(Hwnd,TEST,"!= INVALID_HANDLE_VALUE");
            return false;
        }
        
        if(hDirs == INVALID_HANDLE_VALUE) return false;
        else if(hDirs != INVALID_HANDLE_VALUE)
        {
            while(FindNextFile(hDirs,&Dirs) && GetLastError() != ERROR_NO_MORE_FILES)
            {
                if((Dirs.dwFileAttributes == FILE_ATTRIBUTE_DIRECTORY)){
                    if(!(strcmp(Dirs.cFileName,"..") == 0 || (strcmp(Dirs.cFileName,".") == 0)))
                    {
                        SEARCHFILES pArgs2; //create new structure
                        pArgs2 = pArgs; //assign new structure the old structure
                        pbuffer = _getcwd(lBuffer,MAX_PATH); //get the current directory
                        strcat(pbuffer,Dirs.cFileName); //add the new directory to it
                        pArgs2.StartFolder = pbuffer; //assign new directory that is to be search
                        
                        SetDlgItemText(Hwnd,CUR_PATH,pbuffer); //show the new directory
                        
                        Search(pArgs2,fCount,dCount,Total); //recursively call the function again with a new directory
                    }
                }
            }
        }
        else{
            SetDlgItemText(Hwnd,TEST,"!= INVALID_HANDLE_VALUE");
            return false;
        }
        
        return true;
    }

  2. #2
    Unregistered User Yarin's Avatar
    Join Date
    Jul 2007
    Posts
    2,158
    The problem is that your program is making an invalid memory read/write request.

    On a glance I did notice that your strcat()ing a string the doesn't even look like it has been allocated yet.

  3. #3
    Registered User
    Join Date
    Mar 2007
    Posts
    416
    Quote Originally Posted by Yarin View Post
    The problem is that your program is making an invalid memory read/write request.

    On a glance I did notice that your strcat()ing a string the doesn't even look like it has been allocated yet.
    I fixed the strcat() issue, but where am I making an invalid memory read, and how can I change that? I did not have any read problems before when I was not using recursion.

  4. #4
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    Code:
        LPSTR lBuffer; //storage for _getdcwd
        LPSTR buffer; //displayed storage for _getdcwd
        LPSTR pbuffer; //storage for parent
        LPSTR fBuffer; //storage for file name to be checked
    Windows is evil in using these stupid identifiers. What they are, are pointers. LPSTR is a pointer to char, or in other words, char*.
    So you just have a pointer and not an actual buffer, thus you get an access violation when writing to them.

    You also seem to be mistaken that you need two loops: one for files and one for directories. This is not necessarily true.
    If you specify "*.*" as the mask, then the function will return all files including directories. However, if you specify anything other than "*.*", it will only return the matching files (no directories). This was the reason I used two loops, one to gather files and one to enumerate directories.

    Code:
    //SendMessage(hStatus,SB_SETTEXT,1,(LPARAM)"files searched: %d fCount"); //this might be wrong
    This would obviously be wrong since you don't want to see "files searched: %d fCount" in your dialog. You need to use sprintf or similar functions to create your string buffer first and then send it via SendMessage.

    And as a last note,
    Code:
    SEARCHFILES pArgs
    You don't really need to it that way. You could just pass in the directory to search or such. You don't need to copy my code 100%. The initial reason I used a struct is because I allowed the search to be done in a different thread if desired. And a thread can only take one argument, so a struct it was.
    Last edited by Elysia; 04-01-2008 at 01:06 AM.
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  5. #5
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,660
    > LPSTR lBuffer; //storage for _getdcwd

    Consider the difference between
    char *temp = NULL; strcpy( temp, "hello world" );
    and
    char temp[100]; strcpy( temp, "hello world" );

    As far as strcpy is concerned, the first parameter has the same TYPE in both cases, but the result is very different. It isn't enough to simply declare variables of the type expected by the various functions, you have to think about what the function is going to do with that variable when it gets it. For pointers especially, this means you have to think about where the memory is which is going to get written to, or read from.
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.

  6. #6
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    To expand:
    Virtual memory, the memory a process uses to read and write to, is divided into blocks the size of a byte. To be able to use all the memory, each block needs an address. A virtual address. Now, when you pass something to a function, usually it's by value - a new variable is created and the original value is copied to the new variable.
    But if you want to change the original variable you pass into a function, then you must tell that function the address of the original variable, otherwise it can't find it. And to get the address of a variable, you must use the address of operator (&). If you do not use this, then you are lying to the function and supplying an invalid address, thus causing the whole thing to fall apart.
    Note that arrays are an exception to the address rule. Instead of copying the entire array by value, the compiler instead takes the address where the array begins by default, thus creating a pointer without the need for &.
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  7. #7
    Registered User
    Join Date
    Mar 2007
    Posts
    416
    Alright I'm confused. When I had it the first way (the one in the old thread) I had the LPSTR buffers declared as globals, and it worked fine. I never got any errors, and did not run in to the issue you guys are saying. I think I understand what you guys are saying though. I am declaring a variable, then trying to give it an assignment when really it corresponds to nothing in the first place, right? How should I go about changing this then so it is essentially non-problematic. I don't want to change it, then run in to another different problem that I might think is the same problem.

    Elysia-
    I know I don't need two loops. I could have merged them under the same loop to shorten it up, but if it works I don't see a need to change it. And the thing about the struct as an argument in the function, I realized what you did there and noticed I could maybe use that to give the user extra options of searching later on without having to change the arguments of the function, and just add them to the struct. It was a clever idea to make it short

  8. #8
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    Quote Originally Posted by scwizzo View Post
    Alright I'm confused. When I had it the first way (the one in the old thread) I had the LPSTR buffers declared as globals, and it worked fine. I never got any errors, and did not run in to the issue you guys are saying. I think I understand what you guys are saying though. I am declaring a variable, then trying to give it an assignment when really it corresponds to nothing in the first place, right? How should I go about changing this then so it is essentially non-problematic. I don't want to change it, then run in to another different problem that I might think is the same problem.
    Well, if you re-read my above reply, you would see that you have a pointer pointing to somewhere in memory you don't have write access and therefore you get a crash.

    Elysia-
    I know I don't need two loops. I could have merged them under the same loop to shorten it up, but if it works I don't see a need to change it. And the thing about the struct as an argument in the function, I realized what you did there and noticed I could maybe use that to give the user extra options of searching later on without having to change the arguments of the function, and just add them to the struct. It was a clever idea to make it short
    You do realize that most people prefer to just pass arguments instead of creating a struct and passing it, right?
    The example is an "internal" function. It's not exported publically. There's a public interface that doesn't take a struct that calls this internal function.
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  9. #9
    Registered User
    Join Date
    Mar 2007
    Posts
    416
    Now I'm really lost. If it worked before, and doesn't now, how did I change the write access in going from global to local? Better yet, how can I get write access. I've been googling this and cannot get an answer.

  10. #10
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    Use malloc for C or new for C++, or just make an array on the stack. Don't use LP* types. Windows uses them to indicate a pointer is expected. LPSTR means "LP" - pointer and "STR" - string, or in other words char.
    So something like char[100] or something will work.
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  11. #11
    Registered User
    Join Date
    Mar 2007
    Posts
    416
    I used the new operator, and also tried making char variables with pointers to them separately. Both ways got rid of the error. I just need to work out some other kinks now. Thanks guys!

    EDIT: New problem... the hFiles is returning INVALID_HANDLE_VALUE every time. Does the directory need to be changed in order for FindFirstFile to work?

    Code:
        LPSTR lBuffer = new char[256]; //storage for _getcwd
        LPSTR pbuffer = new char[256]; //storage for parent
        LPSTR fBuffer = new char[256]; //storage for file name to be checked
        
        WIN32_FIND_DATA Files,Dirs; //structure for handling all the files
        HANDLE hFiles = NULL, hDirs = NULL; //handles for FindNextFile()
        
        strcat(fBuffer,pArgs.StartFolder); //set up new folder to search in
        strcat(fBuffer,"\\*.*"); //find all files in new folder
        
        hFiles = FindFirstFile(fBuffer,&Files); //find the first, associated with files
        hDirs = FindFirstFile(fBuffer,&Dirs); //find the first, associated with directories
    
        if(hFiles == INVALID_HANDLE_VALUE){
            MessageBox(Hwnd,"INVALID_HANDLE_VALUE/hFiles","Error",MB_OK);
            return false; //if the file handle is invalid, return false
        }
    EDIT 2: Never mind... it was the strcat() in blue that was giving me the problem. I changed it to strcpy() and it works now.
    Last edited by scwizzo; 04-01-2008 at 02:29 PM.

  12. #12
    Unregistered User Yarin's Avatar
    Join Date
    Jul 2007
    Posts
    2,158
    Don't forget to call "delete [] buffer;" after you are done using them or after or before calling things like _getcwd().

    And even if your code is working fine, it's still good practice to shorten or simplify it afterwards.

  13. #13
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    I think new is overcomplicated in this code. Use a regular char array instead.
    And I would like to say, avoid LPSTR and the like. Use the real type, such as char* instead.
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  14. #14
    Registered User
    Join Date
    Mar 2007
    Posts
    416
    I changed the LPSTR to char*, and the delete []'s are in when the buffers are no longer needed. However, the program gets the same Send Error Report dialog now after going through about 3 or 4 folders. There's also the problem of where the messagebox() in blue is, if that message box is not there it will not go through the directories loop. What's the deal with that?

    Here's the revised code.

    Code:
    bool Search(LPSTR sFolder, int &fC, int &dC, int &total){
        
        char* lBuffer = new char[256]; //storage for _getcwd
        char* pbuffer = new char[256]; //storage for parent
        char* fBuffer = new char[256]; //storage for file name to be checked
        char* buffer = new char[256];
        
        WIN32_FIND_DATA Files,Dirs; //structure for handling all the files
        HANDLE hFiles = NULL, hDirs = NULL; //handles for FindNextFile()
        
        strcpy(fBuffer,sFolder); //set up new folder to search in
        strcat(fBuffer,"\\*.*"); //find all files in new folder
        
        hFiles = FindFirstFile(fBuffer,&Files); //find the first, associated with files
        hDirs = FindFirstFile(fBuffer,&Dirs); //find the first, associated with directories
        
        if(hFiles == INVALID_HANDLE_VALUE || hDirs == INVALID_HANDLE_VALUE){
            SetDlgItemText(Hwnd,TEST,"hFiles/hDirs = INVALID_HANDLE_VALUE");
            return false;
        }
        else if(hFiles != INVALID_HANDLE_VALUE || hDirs != INVALID_HANDLE_VALUE)
        {
            while((FindNextFile(hFiles,&Files) && GetLastError() != ERROR_NO_MORE_FILES) || (FindNextFile(hDirs,&Dirs) && GetLastError() != ERROR_NO_MORE_FILES))
            {
                if(Files.dwFileAttributes != FILE_ATTRIBUTE_DIRECTORY)
                {
                    fCount++;
                   //this is not returning as true
                    if(strcmp("my_test_file.txt",Files.cFileName) == 0){
                        SetDlgItemText(Hwnd,TEST,"File found!");
                        
                        strcpy(FoundLocation,sFolder);
                        strcat(FoundLocation,Files.cFileName);
                        SetDlgItemText(Hwnd,CUR_PATH,FoundLocation);
                        
                    }
    
                }
                
                if((Dirs.dwFileAttributes == FILE_ATTRIBUTE_DIRECTORY) && !(strcmp(Dirs.cFileName,"..") == 0 || (strcmp(Dirs.cFileName,".") == 0)))
                {
                    char* StartFolder = new char[256]; //create new structure
                    
                    strcpy(StartFolder,sFolder);
                    strcat(StartFolder,"\\");
                    strcat(StartFolder,Dirs.cFileName); //add the new directory to it
                    //strcpy(StartFolder,pbuffer); //assign new directory that is to be search
                    
                    dCount++;
                        
                    Search(StartFolder,fCount,dCount,Total); //recursively call the function again with a new directory
                    delete [] StartFolder;
                }
            }
        }
        
        delete [] pbuffer;
        delete [] fBuffer;
        delete [] lBuffer;
        delete [] buffer;
    
        return true;
    }
    EDIT: I changed the code so the file and directory searches are in the same loop, and it fixed the problem. Right now it seems to work correctly, but it doesn't. It does not display that it found my test file, but it's in one of the first directories that it would search. Did I miss something?
    Last edited by scwizzo; 04-01-2008 at 08:57 PM.

  15. #15
    Registered User
    Join Date
    Mar 2005
    Location
    Mountaintop, Pa
    Posts
    1,058
    Unfortunately, I couldn't understand your logic. So, I modified your function as follows:
    Code:
    // How is the total variable used?????
    #include <windows.h>
    #include <stdio.h>
    
    bool Search(LPTSTR sFolder, int &fCount, int &dCount, int &total)
    {
        char *fBuffer = new char[256]; //storage for file name to be checked
    
        WIN32_FIND_DATA Files;  //structure for handling all the files
        HANDLE hFiles = NULL;  //handles for FindNextFile()
        strcpy(fBuffer,sFolder); //set up new folder to search in
        strcat(fBuffer,"\\*.*"); //find all files in new folder
    
        hFiles = FindFirstFile(fBuffer,&Files); //find the first, associated with files
        if(hFiles != INVALID_HANDLE_VALUE)
        {
            sprintf(fBuffer, "%s\\%s", sFolder, Files.cFileName);
            if (Files.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY && 
                (strcmp(Files.cFileName, ".") != 0) &&
                (strcmp(Files.cFileName, "..") != 0)) {
                ++dCount;
                Search(fBuffer, fCount,dCount, total);
            } 
            else {
                if((strcmp(Files.cFileName, ".") != 0) && (strcmp(Files.cFileName, "..") != 0))
    			{
    				++fCount;
                    printf("cFileName : %s\n", Files.cFileName);
    			}	
            }   
            while(FindNextFile(hFiles,&Files) != 0)
            {
                sprintf(fBuffer, "%s\\%s", sFolder, Files.cFileName);
                if (Files.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY && 
                    (strcmp(Files.cFileName, ".") != 0) &&
                    (strcmp(Files.cFileName, "..") != 0))
                {
                    ++dCount;
                    Search(fBuffer, fCount,dCount, total);
                }        
                else
                {
                    if((strcmp(Files.cFileName, ".") != 0) && (strcmp(Files.cFileName, "..") != 0))
                    {
                        ++fCount;
                        printf("cFileName : %s\n", Files.cFileName);
                    }
                }
            }
            FindClose(hFiles );
        }
        delete [] fBuffer;
        return true;
    }
    
    int main(void)
    {
        int iFcount = 0,  iDcount = 0, iTotal = 0; 
        Search("C:\\Utils\\c\\test",iFcount, iDcount, iTotal);
        printf("iFcount = %d iDcount = %d\n", iFcount, iDcount);
        return 0;
    }

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. An error is driving me nuts!
    By ulillillia in forum C Programming
    Replies: 5
    Last Post: 04-04-2009, 09:15 PM
  2. Compiling sample DarkGDK Program
    By Phyxashun in forum Game Programming
    Replies: 6
    Last Post: 01-27-2009, 03:07 AM
  3. Making C DLL using MSVC++ 2005
    By chico1st in forum C Programming
    Replies: 26
    Last Post: 05-28-2008, 01:17 PM
  4. failure to import external C libraries in C++ project
    By nocturna_gr in forum C++ Programming
    Replies: 3
    Last Post: 12-02-2007, 03:49 PM
  5. Stupid compiler errors
    By ChrisEacrett in forum C++ Programming
    Replies: 9
    Last Post: 11-30-2003, 05:44 PM