assignment fails for a FILE* class

Question

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,

casting it as char* and storing size and length before the string — unknown.prince, Sep 23 '17 at 13:27
Consistent indentation will make your code a lot more understandable - and that includes for yourself. — Andrew Henle, Sep 23 '17 at 13:29
You can't cast a `wchar_t *` to a `char *` to convert UTF-16 to ASCII. You have to call a function that would perform the conversion. And why you are accepted UTF-16 text at the interface, if everything's going to be converted to ASCII at the end anyway? — lost_in_the_source, Sep 23 '17 at 13:30
I am not converting it, it is just to store it, I will read (length*size) and cast it back to wchar_t, why to do all that - english is not my primary language nor my os's — unknown.prince, Sep 23 '17 at 13:33
After `this->~file_ptr();` your object *no longer exists*, and `this` points at a raw block of memory, not an object. You may not set its members to anything. Even if you do nothing and C++ automatically calls the destructor again, that's also undefined behavior. Just directly do `fclose(fptr);` instead. — aschepler, Sep 23 '17 at 13:36

user7860670 · Accepted Answer · 2017-09-23T13:43:59.747

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?

Note you should also disable the compiler-generated copy constructor, and then consider implementing a move constructor. — aschepler, Sep 23 '17 at 13:36
What about the safer swap idiom? You are doing something similar, but it's not exactly the same. — synchronizer, Sep 23 '17 at 15:22

assignment fails for a FILE* class

1 Answers1