Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

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);
}
share|improve this question
    
That should work fine. Just out of curiosity, what reasons may there be to prefer cstdio over fstream? –  ronag Sep 8 '11 at 13:45
1  
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. –  Kerrek SB Sep 8 '11 at 13:48
1  
Would unique_ptr not be a better choice? Are you really going to share it? –  Loki Astari Sep 8 '11 at 15:21
    
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 Sep 8 '11 at 15:23
    
You may also want to use a custom deleter for your shared pointer, which will call fclose –  krz-dev Sep 11 '11 at 0:07
show 2 more comments

2 Answers 2

up vote 11 down vote accepted

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.

share|improve this answer
add comment

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.

share|improve this answer
    
That's a nice design. Thanks! (operator ptr() should be const, though, non?) –  Kerrek SB Nov 17 '11 at 16:04
    
@KerrekSB: Yes, probably. –  Jerry Coffin Nov 17 '11 at 16:08
    
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. –  Jonathan Wakely Jul 9 at 12:38
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.