shared_ptr and FILE for wrapping cstdio (update: also dlfcn.h)

Question

Even in the presence of <fstream>, there may be reason for using the <cstdio> file interface. I was wondering if wrapping a FILE* into a shared_ptr would be a useful construction, or if it has any dangerous pitfalls:

#include <cstdio>
#include <memory>
std::shared_ptr<std::FILE> make_file(const char * filename, const char * flags)
{
 std::FILE * const fp = std::fopen(filename, flags);
 return fp ? std::shared_ptr<std::FILE>(fp, std::fclose) : std::shared_ptr<std::FILE>();
}
int main()
{
 auto fp = make_file("hello.txt", "wb");
 fprintf(fp.get(), "Hello world.");
}

Update: I just realized that it is not allowed to fclose a null pointer. I modified make_file accordingly so that in the event of failure there won't be a special deleter.

---

Second update: I also realized that a unique_ptr might be a more suitable than shared_ptr. Here is a more general approach:

typedef std::unique_ptr<std::FILE, int (*)(std::FILE *)> unique_file_ptr;
typedef std::shared_ptr<std::FILE> shared_file_ptr;
static shared_file_ptr make_shared_file(const char * filename, const char * flags)
{
 std::FILE * const fp = std::fopen(filename, flags);
 return fp ? shared_file_ptr(fp, std::fclose) : shared_file_ptr();
}
static unique_file_ptr make_file(const char * filename, const char * flags)
{
 return unique_file_ptr(std::fopen(filename, flags), std::fclose);
}

Edit. Unlike shared_ptr, unique_ptr only invokes the deleter if the pointer is non-zero, so we can simplify the implementation of make_file.

Third Update: It is possible to construct a shared pointer from a unique pointer:

unique_file_ptr up = make_file("thefile.txt", "r");
shared_file_ptr fp(up ? std::move(up) : nullptr); // don't forget to check

---

Fourth Update: A similar construction can be used for dlopen()/dlclose():

#include <dlfcn.h>
#include <memory>
typedef std::unique_ptr<void, int (*)(void *)> unique_library_ptr;
static unique_library_ptr make_library(const char * filename, int flags)
{
 return unique_library_ptr(dlopen(filename, flags), dlclose);
}

• Kerrek SB: To be brutally honest, I have several programs to dissect binary files, so I frequently want to print out some blocks of data in fixed-width hex, and others in decimals, and others in floats, and I'm very happy with printf for that purpose. Attempts to do that in iostreams lead to dramatic amounts of boilerplate code and it's never clear whether something will come out decimal or hex. So fprintf it is :-) But I was just sort of curious in general whether this would be a useful and correct idiom.
• Loki Astari: Would unique_ptr not be a better choice? Are you really going to share it?
• Kerrek SB: Wouldn't being able to share it be cool? Yeah, unique_ptr is certainly an alternative... I just thought of another application: You can put those guys into a standard container and thus manage a collection of open files easily.
• Kerrek SB: I discovered that unique_ptr is better in the sense that it only invokes the deleter if the pointer is not null. Also, you can create a shared pointer from a unique one, but that opens up the problem of null pointer deletion.
• Mast: Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers . Post a follow-up question with both old and new question linking to each other instead.

Answer 1

Honestly, I was thinking very hard to come up with any real disadvantage this might have, but I cannot come up with anything. It certainly look strange to wrap a C structure into a shared_ptr, but the custom deleter takes care of that problem, so it is just a subjective dislike, and only at first. Actually now, I think it is quite clever.

Answer 2

I should start with the fact that I don't entirely agree with the widespread belief that "explicit is better than implicit". I think in this case, it's probably at least as good to have a class that just implicitly converts to the right type:

class file { 
 typedef FILE *ptr;
 ptr wrapped_file;
public:
 file(std::string const &name, std::string const &mode = std::string("r")) : 
 wrapped_file(fopen(name.c_str(), mode.c_str())) 
 { }
 operator ptr() const { return wrapped_file; }
 ~file() { if (wrapped_file) fclose(wrapped_file); }
};

I haven't tried to make this movable, but the same general idea would apply if you did. This has (among other things) the advantage that you work with a file directly as a file, rather than having the ugly (and mostly pointless) .get() wart, so code would be something like:

file f("myfile.txt", "w");
if (!f) {
 fprintf(stderr, "Unable to open file\n");
 return 0;
}
fprintf(f, "Hello world");

This has a couple of advantages. The aforementioned cleanliness is a fairly important one. Another is the fact that the user now has a fairly normal object type, so if they want to use overloading roughly like they would with an ostream, that's pretty easy as well:

file &operator<<(file &f, my_type const &data) { 
 return data.write(f);
}
// ...
file f("whatever", "w");
f << someObject;

In short, if the user wants to do C-style I/O, that works fine. If s/he prefers to do I/O more like iostreams use, a lot of that is pretty easy to support as well. Most of it is still just syntactic sugar though, so it generally won't impose any overhead compare to using a FILE * directly.

• Kerrek SB: That's a nice design. Thanks! (operator ptr() should be const, though, non?)
• Jerry Coffin: @KerrekSB: Yes, probably.
• Jonathan Wakely: Since the original answer goes to great pains to manage the lifetime of the FILE* correctly it seems odd to propose an alternative solution that fails to deal with copying correctly. I might build something like your file on top of the OP's unique_file_ptr or shared_file_ptr, not instead of them.
• Ziezi: Shouldn't the overloaded operator<< be something that includes iostream object as a first parameter and returning value (by reference), and the class file object as a second parameter?
• Jerry Coffin: @simplicisveritatis: Um...what? No, the idea here is that the file type basically acts as a substitute for an iostream, so there's no iostream involved in using it.

Answer 3

Using a function pointer of function reference as a deleter has two major disadvantages:

1. The deleter has state and thus requires storage, even though it is always the same, and the maker function is artificially used to create this "constant" state even though there is no choice to make here.

2. Taking the address of a standard-library function is highly problematic. C++20 has started outright outlawing this practice, and it generally creates brittle code. Functions are first and foremost designed to be called in a certain way. The details of whether a function has overloads, default arguments, etc. are generally not meant to be observable, and liable to change at the whim of the implementer. Standard library functions should therefore always be called.

Putting those two observations together immediately leads to an improved solution: Define your own custom deleter class. This class can be default-constructible, making the smart pointer construction straight-forward.

Example (using dlopen/dlclose):

struct DlCloser
{
 void operator()(void * dlhandle) const noexcept { dlclose(dlhandle); } 
};
using dl_ptr = std::unique_ptr<void, DlCloser>;
dl_ptr make_loaded_dso(const string & filename)
{
 return dl_ptr(dlopen(filename.c_str()));
}

Note that the maker function is now almost useless; I might as well just write dl_ptr p(dlopen(filename)) instead of auto p = make_loaded_dso(filename.c_str()).

Finally, here is a small aside on lambdas: The usual way to use library functions as callbacks and abide by the aforementioned "call-only" interface is to use a lambda expression, such as [](void * h) { dlclose(h); }. However, lambda expressions don't make for good deleter types. Even though C++20 made stateless lambdas default-constructible and allowed lambdas to appear in unevaluated contexts, we cannot generally use something like

std::unique_ptr<void, decltype([](void * h) { dlclose(h); })>

as a library type, since (if the above is contained in a header file) the lambda expression has a unique type in every translation unit and we would therefore have ODR violations. Unevaluated and default-construcible lambdas are only useful in a local setting, but not for libraries and interface types.

• OwnageIsMagic: inline should fix the last problem with ODR violation
• OwnageIsMagic: or static auto deleter = [](void * h) { dlclose(h); }; using ptr = std::unique_ptr<void, decltype(deleter)>;