Directory traversal issue

This is a discussion on Directory traversal issue within the Windows Programming forums, part of the Platform Specific Boards category; I'm trying to write a function that calculates the size of a given directory tree. It's your run-of-the-mill directory traversal ...

  1. #1
    Registered User
    Join Date
    Aug 2006
    Posts
    43

    Directory traversal issue

    I'm trying to write a function that calculates the size of a given directory tree. It's your run-of-the-mill directory traversal that simply adds the sizes of each file. It should be simple enough, but for some reason the function doesn't return the correct size for all folders. I'm not sure if it doesn't report large file sizes correctly or large folder sizes, or maybe it skips some folders - I don't know. Some folders it reports fine, and others not. Any ideas?

    Code:
    long CDirectoryInfo::GetDirectorySize(CString path)
    {
        WIN32_FIND_DATA data;
        HANDLE hFind;
        CString szCurrentPath = path + "\\*.*";
        hFind = FindFirstFile((LPCTSTR)szCurrentPath, &data);
        if(hFind == INVALID_HANDLE_VALUE)
        {
            return -1;
        }
        do
        {
            if(data.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)
            {
                if((strcmp(data.cFileName, ".") != 0) && (strcmp(data.cFileName, "..") != 0))
            {
                szCurrentPath = path + "\\" + data.cFileName;
                GetDirectorySize(szCurrentPath);
    			}
            }
            else
            {
                m_lDirectorySize += data.nFileSizeHigh * (MAXDWORD + 1) + data.nFileSizeLow;
            }
        }
        while(FindNextFile(hFind, &data));
        FindClose(hFind);
    
        return m_lDirectorySize;
    }

  2. #2
    and the hat of wrongness Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    32,334
    You ignore the result of your recursive calls.
    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.
    I support http://www.ukip.org/ as the first necessary step to a free Europe.

  3. #3
    Registered User
    Join Date
    Aug 2006
    Posts
    43

    Recursive calls

    Quote Originally Posted by Salem View Post
    You ignore the result of your recursive calls.
    I'm not sure why that applies. Forgive my ignorance for saying so. As I see it, m_lDirectorySize is a member variable that gets initialized to zero in the class constructor. Then, each time the the function finds a (non-directory) file, it increases m_lDirectorySizeby the size of that file. Why, then, would I need to bother with the intermediate return values? I could instead write the function to not return a value, and write another function, like long GetDirectorySize() that returns m_lDirectorySize. The only thing I can think of is that each time the function calls itself, a "temporary" object of the class is created, reinitializing the value of m_lDirectorySizeto zero. If that's the case, would I declare m_lDirectorySize static?

  4. #4
    and the hat of wrongness Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    32,334
    OK, so why do you have
    return m_lDirectorySize;
    at the end then?

    Either it always returns the result and you don't need a member variable, or it never returns the result and you have another member function to retrieve the total. Choose one (it doesn't matter), but don't have sloppy interfaces like this.

    The next step would be say
    Code:
    long CDirectoryInfo::GetDirectorySize(CString path)
    {
        WIN32_FIND_DATA data;
        HANDLE hFind;
        CString szCurrentPath = path + "\\*.*";
        hFind = FindFirstFile((LPCTSTR)szCurrentPath, &data);
        if(hFind == INVALID_HANDLE_VALUE)
        {
            return -1;
        }
        do
        {
            if(data.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)
            {
                if((strcmp(data.cFileName, ".") != 0) && (strcmp(data.cFileName, "..") != 0))
                {
                    szCurrentPath = path + "\\" + data.cFileName;
                    cout << "Recursing to " << szCurrentPath;
                    GetDirectorySize(szCurrentPath);
                }
            }
            else
            {
                m_lDirectorySize += data.nFileSizeHigh * (MAXDWORD + 1) + data.nFileSizeLow;
                cout << "Total size currently " << m_lDirectorySize;
            }
        }
        while(FindNextFile(hFind, &data));
        FindClose(hFind);
    
        return m_lDirectorySize;
    }
    Add some debug to the code, and watch what happens.
    Or maybe use the debugger to put breakpoints on those locations, and watch the variables.

    > The only thing I can think of is that each time the function calls itself,
    > a "temporary" object of the class is created,
    So test your idea with some debug in the constructor.

    Are there very large files?
    On the face of it, your size calculation overflows.
    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.
    I support http://www.ukip.org/ as the first necessary step to a free Europe.

  5. #5
    C++まいる!Cをこわせ! Elysia's Avatar
    Join Date
    Oct 2007
    Posts
    22,167
    Quote Originally Posted by Mostly Harmless View Post
    Code:
            if(data.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)
            {
                if((strcmp(data.cFileName, ".") != 0) && (strcmp(data.cFileName, "..") != 0))
            {
                szCurrentPath = path + "\\" + data.cFileName;
                GetDirectorySize(szCurrentPath);
    			}
            }
    Salem fixes your little indentation "problem."
    Avoid from doing that, it makes the code look sloppy, ugly and it gets harder to read.
    Tabs are better than spaces too, if you ask me.
    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.
    For information on how to enable C++11 on your compiler, look here.
    よく聞くがいい!私は天才だからね! ^_^

  6. #6
    Registered User
    Join Date
    Mar 2005
    Location
    Mountaintop, Pa
    Posts
    1,059
    It looks like you're not processing the results of your FindFirstFile call:

    Code:
    long CDirectoryInfo::GetDirectorySize(CString path)
    {
        WIN32_FIND_DATA data;
        HANDLE hFind;
        CString szCurrentPath = path + "\\*.*";
        hFind = FindFirstFile((LPCTSTR)szCurrentPath, &data);
        if(hFind == INVALID_HANDLE_VALUE)
        {
            return -1;
        }
    	 if (data.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY && 
    		(strcmp(data.cFileName, ".") != 0) &&
    		(strcmp(data.cFileName, "..") != 0))
    	{
    		szCurrentPath = path + "\\" + data.cFileName;
    		GetDirectorySize(szCurrentPath);
    	} 
    	else
    	{
    		if((strcmp(data.cFileName, ".") != 0) && (strcmp(data.cFileName, "..") != 0))
    		{
    			m_lDirectorySize += data.nFileSizeHigh * (MAXDWORD + 1) + data.nFileSizeLow;
    		}
    	}   
        do
        {
            if(data.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)
            {
                if((strcmp(data.cFileName, ".") != 0) && (strcmp(data.cFileName, "..") != 0))
            {
                szCurrentPath = path + "\\" + data.cFileName;
                GetDirectorySize(szCurrentPath);
    			}
            }
            else
            {
                m_lDirectorySize += data.nFileSizeHigh * (MAXDWORD + 1) + data.nFileSizeLow;
            }
        }
        while(FindNextFile(hFind, &data));
        FindClose(hFind);
    
        return m_lDirectorySize;
    }

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Directory traversal problem
    By phlook in forum C Programming
    Replies: 2
    Last Post: 03-16-2009, 12:31 AM
  2. Recursive Directory Traversal difficulties
    By phlook in forum C Programming
    Replies: 3
    Last Post: 03-15-2009, 07:26 PM
  3. Profiler Valgrind
    By afflictedd2 in forum C++ Programming
    Replies: 4
    Last Post: 07-18-2008, 09:38 AM
  4. Couple errors please help :-D
    By JJJIrish05 in forum C Programming
    Replies: 9
    Last Post: 03-06-2008, 01:54 AM
  5. Request for comments
    By Prelude in forum A Brief History of Cprogramming.com
    Replies: 15
    Last Post: 01-02-2004, 09:33 AM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21