Labels for premature function exit + cleanup

This is a discussion on Labels for premature function exit + cleanup within the C++ Programming forums, part of the General Programming Boards category; Just wondering how you guys feel about this sort of thing: Code: template<typename T> File<T>* FileSystem::Open( const char* szName, int ...

  1. #1
    Supermassive black hole cboard_member's Avatar
    Join Date
    Jul 2005
    Posts
    1,709

    Labels for premature function exit + cleanup

    Just wondering how you guys feel about this sort of thing:

    Code:
    template<typename T> File<T>* FileSystem::Open( const char* szName, int iMode )
    {
        FILE*       fRead = 0;
        File<T>*    fCreated = new File<T>;
        int         iAbsPathLen = strlen( szName ) + strlen( g_szDataRoot ) + 1;
        char*       szAbsPath = new char[iAbsPathLen];
    
        sprintf( szAbsPath, "%s%s", g_szDataRoot, szName );
    
        if ( iMode & FS_OPEN_READ )
            fRead = fopen( szAbsPath, "rb" );
        else if ( iMode & FS_OPEN_WRITE )
            fRead = fopen( szAbsPath, "wb" );
        else
        {
            Error::GetSingleton().Log( true, "FileSystem::Open: Invalid mode" );
            delete[] szAbsPath;
            delete fCreated;
            return 0;
        }
    
        if ( !fRead )
        {
            Error::GetSingleton().Log( true, "FileSystem::Open: Unable to open '%s'", szAbsPath );
            delete[] szAbsPath;
            delete fCreated;
            return 0;
        }
    
    }
    To me the two error situations duplicating the same cleanup code seems a bit, well, pointless. I've seen something along the lines of this being done (mainly in the Linux source code):

    Code:
    template<typename T> File<T>* FileSystem::Open( const char* szName, int iMode )
    {
        FILE*       fRead = 0;
        File<T>*    fCreated = new File<T>;
        int         iAbsPathLen = strlen( szName ) + strlen( g_szDataRoot ) + 1;
        char*       szAbsPath = new char[iAbsPathLen];
    
        sprintf( szAbsPath, "%s%s", g_szDataRoot, szName );
    
        if ( iMode & FS_OPEN_READ )
            fRead = fopen( szAbsPath, "rb" );
        else if ( iMode & FS_OPEN_WRITE )
            fRead = fopen( szAbsPath, "wb" );
        else
        {
            Error::GetSingleton().Log( true, "FileSystem::Open: Invalid mode" );
            goto exitFail;
        }
    
        if ( !fRead )
        {
            Error::GetSingleton().Log( true, "FileSystem::Open: Unable to open '%s'", szAbsPath );
            goto exitFail;
        }
    
        // Return successfully here...
    
    exitFail:
        delete[] szAbsPath;
        delete fCreated;
        return 0;
    }
    Thoughts?
    Good class architecture is not like a Swiss Army Knife; it should be more like a well balanced throwing knife.

    - Mike McShaffry

  2. #2
    Registered User Kurisu's Avatar
    Join Date
    Feb 2006
    Posts
    62
    Just wondering how you guys feel about this sort of thing:
    Labels make me uneasy in higher level programming languages as they are rarely encountered; atleast in my experience.

    I'd probably go with the first one just because.. uh.. i dunno... I wonder if the compiler would adequately optimize the code to avoid redundancy so that both your code samples would be equivalent after compilation?..?..?

    Code:
    1: ifnot() jmp 4
    2: Error::GetSingleton().Log( true, "FileSystem::Open: Invalid mode" );
    3: jmp 6;
    4: if(fRead) jmp 8;
    5: Error::GetSingleton().Log( true, "FileSystem::Open: Unable to open '%s'", szAbsPath );
    6: delete[] szAbsPath;
    7: delete fCreated;
    8: return 0;
    Kinda curious on others' opinions as in this case the goto label logic is clear in your code, which anyone should understand.
    Last edited by Kurisu; 06-12-2006 at 01:35 PM.

  3. #3
    Even death may die... Dante Shamest's Avatar
    Join Date
    Apr 2003
    Location
    Malaysia
    Posts
    970
    I'd choose the latter (the 2nd example), but only for C code, not C++ code.

    If I was writing C++, I'd prefer to put new/delete allocations in a class of some sort using the RAII idiom, so that when I define a variable in a function, I don't have to worry about cleaning it up (just like if you forget to call close() for ifstream, it'll close itself once it goes out of scope).

  4. #4
    Registered User
    Join Date
    Jan 2005
    Posts
    7,317
    Using RAII is probably the best way to go. You can also separate that code into another function that returns false on error, then do your error handling in one place if it does.

    In this example, szAbsPath could be a string or vector<char>. Also, it appears that fCreated doesn't need to be created yet, so you could initialize it to 0 and create it later when you know it will be valid.

  5. #5
    Registered User
    Join Date
    Apr 2006
    Posts
    2,009
    Yep, smart pointers, such as auto_ptr are the way to go.
    It is too clear and so it is hard to see.
    A dunce once searched for fire with a lighted lantern.
    Had he known what fire was,
    He could have cooked his rice much sooner.

  6. #6
    Supermassive black hole cboard_member's Avatar
    Join Date
    Jul 2005
    Posts
    1,709
    Okey dokey.
    Good class architecture is not like a Swiss Army Knife; it should be more like a well balanced throwing knife.

    - Mike McShaffry

  7. #7
    Cat without Hat CornedBee's Avatar
    Join Date
    Apr 2003
    Posts
    8,892
    For example, if you must use C-style file handling (in C++, the C++ iostreams should be preferred), you can easily use Boost.shared_ptr to manage the handle:
    Code:
    boost::shared_ptr<FILE> handle(fopen(filename, mode), fclose);
    fread(handle.get(), ...);
    The file is automatically closed when handle goes out of scope (unless a copy was made).
    All the buzzt!
    CornedBee

    "There is not now, nor has there ever been, nor will there ever be, any programming language in which it is the least bit difficult to write bad code."
    - Flon's Law

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Getting an error with OpenGL: collect2: ld returned 1 exit status
    By Lorgon Jortle in forum C++ Programming
    Replies: 6
    Last Post: 05-08-2009, 08:18 PM
  2. Seg Fault in Compare Function
    By tytelizgal in forum C Programming
    Replies: 1
    Last Post: 10-25-2008, 03:06 PM
  3. We Got _DEBUG Errors
    By Tonto in forum Windows Programming
    Replies: 5
    Last Post: 12-22-2006, 04:45 PM
  4. Dikumud
    By maxorator in forum C++ Programming
    Replies: 1
    Last Post: 10-01-2005, 06:39 AM
  5. Replies: 5
    Last Post: 02-08-2003, 06:42 PM

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