Stroustrup's RAII and the cast operator FILE* () = contradiction?

Question

I was reading through Stroustrup’s C++ (3ed, 1997) to see how he implemented the RAII, and on page 365 I found this:

class File_ptr{
    FILE* p;
public:
    File_ptr(const char* n, const char* a){p = fopen(n, a);}
    File_ptr(FILE* pp) { p = pp; }
    ~File_ptr() {fclose(p);}
    operator FILE* () {return p;}
};

The implementation of constructors and destructors is obvious and complies with the RAII idiom, but I don’t understand why he uses the operator FILE* () {return p;}.

This would result in using File_ptr in the following way:

FILE* p = File_ptr("myfile.txt", "r");

The result in a closed p, which is semantically inappropriate in this case. Also, if File_ptr is meant to be used as RAII, this operator allows it to be misused like in the example. Or am I missing something?

Larrylars

I think the point of that operator is so that you can pass a File_ptr to functions such as fputs that accept a FILE *, not a File_ptr. Yes, this definition allows it to be used incorrectly. – Minhminho

That fact that the file is closed in your example is exactly the point of RAII. File_ptr is responsible for this resource over its entire lifetime. – Instalment

You're correct. This is one of the reasons std::string doesn't have an operator char const*(). – Zamir

You should take a look at unique_ptr, which is a modern (C++ 11) class with similar semantics. It uses a method called get() in this context. – Sacramentalist

Its not about the operator or a member function. You can still write, const char* val = std::string("blah").c_str(). – Hephzibah

The difference is significant, because with the operator chances are much higher that this happens accidentally. – Natka

Indeed. The fact that this can happen accidentally is exactly what I was alluding to in my question. That's why I find this &-thingy so nice. (see below in Mikhail answer.) – Larrylars

Answer 1

Seems like it is inevitable evil price for comfort. Once you want a way FILE* be extractible from your fancy RAII class, it can be misused. Will it be operator FILE*() or FILE* getRawPtr() method, or whatever, it can be called on a temporarty object, making the result invalid just after it is returned.

In C++11, however, you can make this a little more secured, by disallowing this call on temporaries, like this:

operator FILE* () & { return p; }
// Note this -----^

Useful link on how it works provided by Morwenn in the comments: What is "rvalue reference for *this"?

Rhachis

This is the first time I see something about disabling an operator for temporaries. Can you please provide me with any reference or search term for finding more details? Thanks. – Cystotomy

@Cystotomy That's rvalue references for *this. – Oversubscribe

This will disallow usage like fgetc(File_ptr("my_file")) Which might be useful – Hephzibah

Answer 2

Thinking has moved on quite a way from 1997 as a result of experience, and one of the major recommendations now is not to have implicit cast operators because of problems like this. It was previously thought better to have an implicit conversion operator to make it easier to retrofit to existing code, but this led to problems when the function destroys the resource, as the RAII wrapper class doesn't know about it.

The modern convention is to provide access to the underlying pointer but to give it a name so that at least it's explicit. It won't catch every possible problem but makes it easier to grep for potential violations. For instance with std::string it's c_str():

std::string myString("hello");
callFunctionTakingACharPointer(myString.c_str());
// however...
delete myString.c_str();  // there's no way of preventing this

Woodberry

technically it's data() but c_str() does that same in C++11 – Ify

Really? I've been using c_str() for years on multiple platforms – Woodberry

yeah but data() is not guaranteed to be null terminated in C++98 so c_str could cause a realloc – Ify

@ratchet: Is there an actual implementation that does not save the string in null-terminated representation already? It seems like a very reasonable thing to do since the overhead is so low – Hairpiece

@NiklasB. if you allow the array to be shared to quicker substring allocation but that create unpredictable timings when asking the c_str and/or manipulate it – Ify

@ratchetfreak: But as I understand it sharing is also something that is not often done in practice, because it tends to create memory leakes. – Hairpiece

Answer 3

I don’t understand why he uses the operator FILE* () {return p;}.

The reason for the operator is to provide access/compatibility for APIs that use a FILE*. The problem with the implementation is that it allows client code similar to what you gave as an example.

This would result in using File_ptr in the following way:

FILE* p = File_ptr("myfile.txt", "r");

No actually. While you can do this, the class definition doesn't result in code like this. It is up to you to avoid writing this kind of code (just as it is normally up to you to write code that avoids life-time management issues).

The RAII example in your question is an example of poor design. The conversion operator could be avoided.

I would replace it with a FILE *const File_ptr::get() const accessor. That change doesn't eliminate the problem, but it makes it easier to see in client code that you are returning a const pointer (i.e. "ClientCode, please don't delete this"), from a class.

Cystotomy

Answer 4

that's not following the rule of 3 (let alone 5),

so declaring a function as Bar* createBarFromFile(File_ptr ptr) will do unexpected things (the file will be closed after calling this function)

it needs to define a copy constructor and a copy assignment constructor. for rule of 5 it also needs the move variants

however if I'm forced to use a FILE* as member fields I prefer to use a std::unique_ptr<FILE, int (__cdecl *)(FILE *)> and pass &fclose in the constructor

Ify

+1 for std::unique_ptr<FILE, int (__cdecl *)(FILE *)>. I've been doing this myself, but I wasn't sure if it was a practice. – Supremacist

@Kivin another way is to create a stateless FileDeleter class with a void operator()(FILE* f){fclose(f);] and use that as the second template parameter, upside of that is that you can typedef it without needing to fiddle with the constructor – Ify