views:

290

answers:

2

I've read the article about scope guards (Generic: Change the Way You Write Exception-Safe Code — Forever) in DDJ and I understand their common use.

However, the common use is to instantiate a particular stack guard on the stack for a particular operation, e.g.:

{
    FILE* topSecret = fopen("cia.txt");
    ON_BLOCK_EXIT(std::fclose, topSecret);
    ... use topSecret ...
} // topSecret automagically closed

but what if I want to schedule cleanup operations in runtime, e.g. when I have a loop:

{
   vector<FILE*> topSecretFiles;
   for (int i=0; i<numberOfFiles; ++i)
   {
      char filename[256];
      sprintf(filename, "cia%d.txt", i);
      FILE* topSecret = fopen(filename);
      topSecretFiles.push_back(topSecret);
      ON_BLOCK_EXIT(std::fclose, topSecret); // no good
   }
}

Obviously, the above example wouldn't work, since topSecret would be closed along with the for scope. I'd like a scope guard pattern where I can just as easily queue up cleanup operations which I determine to be needed at runtime. Is there something like this available?

I can't push scope guard objects into a standard queue, cause the original object (the one I'm pushing) would be dismissed in the process. How about pushing heap-allocated stack guards and using a queue which deletes its members on dtor? Does anyone have a more clever approach?

+4  A: 

It seems you don't appreciate RAII for what it is. These scope guards are nice on occasion for local ("scope") things but you should try to avoid them in favour of what RAII is really supposed to do: encapsulating a resource in an object. The type FILE* is really just not good at that.

Here's an alternative:

void foo() {
    typedef std::tr1::shared_ptr<FILE> file_sptr;
    vector<file_sptr> bar;
    for (...) {
        file_sptr fsp ( std::fopen(...), std::fclose );
        bar.push_back(fsp);
    }
}

Or:

void foo() {
    typedef std::tr1::shared_ptr<std::fstream> stream_sptr;
    vector<stream_sptr> bar;
    for (...) {
        file_sptr fsp ( new std::fstream(...) );
        bar.push_back(fsp);
    }
}

Or in "C++0x" (upcoming C++ standard):

void foo() {
    vector<std::fstream> bar;
    for (...) {
        // streams will become "movable"
        bar.push_back( std::fstream(...) );
    }
}

Edit: Since I like movable types in C++0x so much and you showed interest in it: Here's how you could use unique_ptr in combination with FILE* without any ref-counting overhead:

struct file_closer {
    void operator()(FILE* f) const { if (f) std::fclose(f); }
};

typedef std::unique_ptr<FILE,file_closer> file_handle;

file_handle source() {
    file_handle fh ( std::fopen(...) );
    return fh;
}

int sink(file_handle fh) {
    return std::fgetc( fh.get() );
}

int main() {
    return sink( source() );
}

(untested)

Be sure to check out Dave's blog on efficient movable value types

sellibitze
Yes, you are right that RAII is preferable for a destructor of a resource (e.g. a file handle). I'm only using scope guards for things which'd be awkward to represent as a "resource", e.g.report(OperationStart);ON_BLOCK_EXIT(report, OperationEnd);doSomething();
Ilya
Regretfully I don't have TR1 in my compiler yet, so I can't use shared_ptr. However, a vector/queue of auto_ptr<ScopeGuard> might just work (assuming I'd be heap-allocating my scope guards and pushing them into the vector/queue). I'm intrigued to learn that some C++ classes will become "movable" (wasn't aware of this terminology). Is this done with refcounts? Maybe I should make the scope guards "movable" too?
Ilya
A vector/queue of auto_ptr<...> is clearly not a good idea, as auto_ptrs are not copyable!
Xavier Nodet
You could use Boost's implementation of shared_ptr instead. A "movable" type is not implemented by indirection and ref-counting. It requires a new C++0x feature: rvalue references. For an in-depth article series on "move semantics" check out http://cpp-next.com/archive/category/value-semantics/ . My suggestion would be to (a) redesign, (b) use Boost's shared_ptr, or (c) write your own RAII-handle class for holding a FILE*.
sellibitze
boost has a version of tr1. Why not just add that to your system. shared_ptr is header only so there is not need to build anything in boost just use the header files.
Martin York
A: 

Huh, turns out the DDJ scope guard is "movable", not in the C++0x sense, but in the same sense that an auto_ptr is movable: during the copy ctor, the new guard "dismisses" the old guard (like auto_ptr's copy ctor calls the old one's auto_ptr::release).

So I can simply keep a queue<ScopeGuard> and it'll work:

queue<ScopeGuard> scopeGuards;

// ...

for (...)
{
   // the temporary scopeguard is being neutralized when copied into the queue,
   // so it won't cause a double call of cleanupFunc
   scopeGuards.push_back(MakeScopeGuard(cleanupFunc, arg1));
   // ...
}

By the way, thank you for the answer above. It was informative and educational to me in different ways.

Ilya
Yes. Its copy constructor "moves" which makes it also as unsafe as auto_ptr. For a C++0xified version check out my blog post: http://pizer.wordpress.com/2008/11/22/scope-guards-revisited-c0x-style/
sellibitze
Simply don't do it! It's wrong. It's bad. It won't work. The guard object they show in that DDJ article is as bad as auto_ptr. It moves on a copy. That's not how a value type should behave!
sellibitze
Also, ScopeGuard ist just a typedef for "ScopeGuardBase const-)
sellibitze
sellibitze, I'm not really handling FILE pointers and stubbornly refusing to use file streams, you know :-) I've just given it for sake of an example. What I'm doing is more along the lines of sendReport(OperationFinished).
Ilya
I'll switch to c++0x when possible; I'm still on MSVC++9. Why do you see them as unsafe? auto_ptrs might have the semantics of a pointer and thus misleading, but this scope guard business is a very particular language extension: you don't really expect it to behave like a civilized value type. Actually, you'd be a fool to expect *anything* off it without inspecting its code throughly.
Ilya
Every type that has a destructive "copy" c'tor is dangerous/unsafe. For a better alternative to std::auto_ptr see std::unique_ptr (C++0x). For a better alternative to the DDJ article's scopeguard object, see my blog post I linked from above. It's safer because in case you try to copy these objects the compiler will complain about the types not being copiable. If you WANT TO MOVE such an object you'll have to use a special syntax for that. --> No accidental moving.
sellibitze