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?