> > Christophe Fergeau writes: > > > On Wed, Nov 08, 2017 at 03:02:35PM +0000, Frediano Ziglio wrote: > >> This better integrate with exceptions. > >> Also don't leak resources using a return in the middle of the > >> code. > >> > >> Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx> > >> --- > >> src/Makefile.am | 1 + > >> src/defer.hpp | 16 ++++++++++++++++ > >> src/spice-streaming-agent.cpp | 16 ++++++++-------- > >> 3 files changed, 25 insertions(+), 8 deletions(-) > >> create mode 100644 src/defer.hpp > >> > >> diff --git a/src/Makefile.am b/src/Makefile.am > >> index 8d5c5bd..ec1969a 100644 > >> --- a/src/Makefile.am > >> +++ b/src/Makefile.am > >> @@ -56,4 +56,5 @@ spice_streaming_agent_SOURCES = \ > >> mjpeg-fallback.cpp \ > >> jpeg.cpp \ > >> jpeg.hpp \ > >> + defer.hpp \ > >> $(NULL) > >> diff --git a/src/defer.hpp b/src/defer.hpp > >> new file mode 100644 > >> index 0000000..93931fe > >> --- /dev/null > >> +++ b/src/defer.hpp > >> @@ -0,0 +1,16 @@ > >> +// see > >> https://stackoverflow.com/questions/32432450/what-is-standard-defer-finalizer-implementation-in-c > > > > Is there any license associated with that code snippet? > > That code snippet and variants thereof have been floating around for > quite a while. Here is a discussion of this topic I found in 2012: > > http://the-witness.net/news/2012/11/scopeexit-in-c11/ > > In one of the comments, there is something that is really close. > > If we want to invent our own by fear of licensing issues, I would go for > something like this: > > struct Defer { > template <typename Action> struct Cleanup > { > ~Cleanup() { action(); } > Action action; > }; > template <typename Action> > friend inline Cleanup<Action> operator >> (Defer, Action action) > { > return Cleanup<Action> { action }; > } > > }; > #define my_defer auto _cleanup_ = Defer{} >> [&]() -> void > > Compared to the original, changes are somewhat minor, but I find them useful: > > 1. There is no "dummy" class. Whatever is returned is a local > Defer::Cleanup class. If we ever need to run that code in a debugger, I > think it will be more explicit where things are coming from. > > 2. I tried to make the code slightly more "readable", e.g. by naming > things a bit better, and using operator>> which is more visible in the > code than operator*. > > 3. I use friend name injection to make all the definitions "fit" inside > Defer. > Yes, more readable than original. > 4. I avoid the macro drudgery. Downside is you have only one defer per > scope. Upside is you have only one defer per scope. I see that as an > upside. > I consider a downside, if I have for instance 2 C open calls and want to define 2 defers I cannot. But back to the license "issue" (I don't see it considering the license) somebody could consider your suggestion inspired by the previous ones so subject to OpenStack licensing too. > > > > > Christophe > > > >> +#ifndef defer > >> +template <class F> struct deferrer > >> +{ > >> + F f; > >> + ~deferrer() { f(); } > >> +}; > >> +struct defer_dummy {}; > >> +template <class F> inline deferrer<F> operator*(defer_dummy, F f) > >> +{ > >> + return {f}; > >> +} > >> +#define DFRCAT_(LINE) _defer##LINE > >> +#define DFRCAT(LINE) DFRCAT_(LINE) > >> +#define defer auto DFRCAT(__LINE__) = defer_dummy{} *[&]() -> void > >> +#endif > >> diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp > >> index ed7ddb9..4122eee 100644 > >> --- a/src/spice-streaming-agent.cpp > >> +++ b/src/spice-streaming-agent.cpp > >> @@ -33,6 +33,7 @@ > >> > >> #include "hexdump.h" > >> #include "concrete-agent.hpp" > >> +#include "defer.hpp" > >> > >> using namespace std; > >> using namespace SpiceStreamingAgent; > >> @@ -353,12 +354,17 @@ do_capture(const char *streamport, FILE *f_log) > >> // TODO was syslog(LOG_ERR, "Failed to open %s: %s\n", > >> streamport, strerror(errno)); > >> throw std::runtime_error("failed to open streaming device"); > >> > >> + defer { > >> + close(streamfd); > >> + streamfd = -1; > >> + }; > >> + > >> unsigned int frame_count = 0; > >> while (! quit) { > >> while (!quit && !streaming_requested) { > >> if (read_command(1) < 0) { > >> syslog(LOG_ERR, "FAILED to read command\n"); > >> - goto done; > >> + return; > >> } > >> } > >> > >> @@ -409,19 +415,13 @@ do_capture(const char *streamport, FILE *f_log) > >> //usleep(1); > >> if (read_command(0) < 0) { > >> syslog(LOG_ERR, "FAILED to read command\n"); > >> - goto done; > >> + return; > >> } > >> if (!streaming_requested) { > >> capture->Reset(); > >> } > >> } > >> } > >> - > >> -done: > >> - if (streamfd >= 0) { > >> - close(streamfd); > >> - streamfd = -1; > >> - } > >> } > >> > >> #define arg_error(...) syslog(LOG_ERR, ## __VA_ARGS__); Frediano _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel