bluedragon bluedragon - 1 month ago 14
C++ Question

assignment fails for a FILE* class

I tried to wrap class around FILE*, here it is

class file_ptr
{
public:
file_ptr(const wstring& _FileN, const wstring& _OpenMode) : file_n(_FileN), omode(_OpenMode),
fptr(_wfopen(file_n.c_str(), omode.c_str()))
{
if (!fptr)
throw wstring(L"Failed to open File ") + _FileN;
}

~file_ptr()
{
fclose(fptr);
}

file_ptr& operator =(const file_ptr& other)
{
if (this != &other)
{
this->~file_ptr();
fptr = other.fptr;
file_n = other.file_n; omode = other.omode;
}
}

operator FILE* () { return fptr; }

private:
wstring file_n, omode;
FILE* fptr;
};


why wstring? I need Unicode support.

now the problem lets say it did something like this

int main() {
try {
file_ptr file1(L"text1",L"wb");
fwrite("Hello",1,5,file1);
file1 = file_ptr(L"text2",L"wb");
fwrite("Hello",1,5,file1);
} catch ( const wstring& e ) {
wcout << e << endl;
}
return 0;
}


Nothing will be written in text2

I even tried after removing my assignment overload, becoz I suppose the default behaviour should be same, but the problem persists

it works if I use raw FILE* as expected f.e

int main() {
try {
FILE* file1 = _wfopen(L"text1",L"wb");
fwrite("Hello",1,5,file1);
fclose(file1);
file1 = _wfopen(L"text2",L"wb");
if (!(file1))
throw L"Can't open file";
fwrite("Hello",1,5,file1);
} catch ( const wstring& e ) {
wcout << e << endl;
}
return 0;
}


text2 is written correctly,

VTT VTT
Answer Source

file1 = file_ptr(L"text2",L"wb"); expression creates a temp file_ptr object and then fptr = other.fptr; copies a FILE pointer value owned by temp object. Temp object gets destroyed immediately and closes file pointer leaving file1 with a dandling FILE pointer. You should write a move assignment operator instead:

file_ptr &
operator =(const file_ptr & other) = delete; // prevent accidental use

file_ptr &
operator =(file_ptr && other) noexcept
{
    if(this == ::std::addressof(other))
    {
        ::std::terminate(); // there is no context when selfassignment makes sense
    }
    //this->~file_ptr(); calling destructor on itself is no good
    ::fclose(this->fptr);
    this->fptr = other.fptr;
    other.fptr = 0;
    this->file_n = ::std::move(other.file_n);
    this->omode = ::std::move(other.omode);
    return(*this);
}

As mentioned in comments, it would be a good idea to disable copy constructor and implement move constructor to prevent similar problems occurring during construction. You may also want to check Rule-of-Three becomes Rule-of-Five with C++11?