File stream functions

Is this good or not? Input arguments for OsFile_Open must be in UTF8 format.

```
#include

#ifdef WIN32
#include
#endif

// Types declaration, originally they are located in header file
//
typedef int Bool_T;
typedef signed long long Int64_T;

#define true 1
#define false 0

#define IN
#define OUT

Bool_T OsFile_Open( IN const char name, IN const char mode, OUT FILE ** file )
{
FILE * handle;

#ifdef WIN32

wchar_t unicodeMode[MAX_PATH];
wchar_t unicodeName[MAX_PATH];

int unicodeNameLen;
int unicodeModeLen;

unicodeNameLen = MultiByteToWideChar( CP_UTF8, 0, name, -1, unicodeName, 0 );

if( unicodeNameLen != 0 )
{
if( MultiByteToWideChar( CP_UTF8, 0, name, -1, unicodeName, unicodeNameLen ) != 0 )
{
unicodeModeLen = MultiByteToWideChar( CP_UTF8, 0, mode, -1, unicodeMode, 0 );

if( unicodeModeLen != 0 )
{
if( MultiByteToWideChar( CP_UTF8, 0, mode, -1, unicodeMode, unicodeModeLen ) != 0 )
{
handle = _wfopen( unicodeName, unicodeMode );

if( handle != NULL )
{
*file = handle;
return true;

}else{

return false;
}

}else{

return false;
}

}else{ // if( unicodeModeLen != 0 )

return false;
}

}else{ // if( MultiByteToWideChar( CP_UTF8, 0, name, -1, unicodeName, unicodeNameLen ) != 0 )

return false;
}

}else{ // if( unicodeNameLen != 0 )

return false;
}

#else

handle = fopen64( name, mode );

if( handle != NULL )
{
*file = handle;
return true;

}else{

return false;
}

#endif

}

Bool_T OsFile_Close( IN FILE * file )
{
return ( fclose( file ) == 0 ) ? true :

I'd re-structure (the Win32 part of) the open function to something like this:

#ifdef WIN32

    wchar_t unicodeMode[MAX_PATH];
    wchar_t unicodeName[MAX_PATH];

    if (MultiByteToWideChar( CP_UTF8, 0, name, -1, unicodeName, MAX_PATH) && 
        MultiByteToWideChar( CP_UTF8, 0, mode, -1, unicodeMode, MAX_PATH) && 
        (handle = _wfopen( unicodeName, unicodeMode)) != NULL) 
    {
        *file = handle;
        return true;
    }
    return false;

#else

You don't need to call MultiByteToWideChar twice for each conversion -- the first call not only returns the length, but does the conversion as well. As it stands, your code has potential buffer overruns as well -- it used the first call to MultiByteToWideChar to measure the length that would be needed for the result, but never checked that this was less than or equal to the buffer space you provided.

It can be legitimate to do the two consecutive calls, especially if (for example) you want to ensure you can handle any possible input by using the first call the measure the necessary length, then allocating the necessary space dynamically, and then doing the second call using the dynamically allocated space.

Several places, you have things like:

return ( fclose( file ) == 0 ) ? true : false;

These should be written as:

return fclose(file) == 0;

or just:

return !fclose(file);

There's no need to select and return a Boolean literal -- the Boolean values yielded by == and ! will work nicely as-is.

Edit: You also have a couple of things like:

#ifdef WIN32
    *curPos = _ftelli64( file );
#else
    *curPos =  ftello64( file );
#endif

spread throughout the code. Where you're just dealing with different function names like this, I think I'd have one block up-front to deal with the name differences, and then use a single name through the rest of the code:

#ifdef WIN32
    #define ftello64(x) _ftelli64(x)
    #define fseeko64(x) _fseeki64(x)
#endif

/* ... */

Bool_T OsFile_Seek( IN FILE * file, IN Int64_T offset, IN int origin )
{
    return !fseeko64(file, offset, origin);
}

You might want to read #ifdef considered harmful for more about what to do and what to avoid.

StackExchange Code Review Q#3309, answer score: 5