[Spice-devel] [PATCH spice-streaming-agent] Use RAII to cleanup stream in case of exception or return

[Frediano Ziglio]

> 
> From: Christophe de Dinechin <dinechin at redhat.com>
> 
> This lets us get rid of C-style 'goto done' in do_capture.
> 
> Signed-off-by: Christophe de Dinechin <dinechin at redhat.com>
> ---
>  src/spice-streaming-agent.cpp | 34 ++++++++++++++++++++--------------
>  1 file changed, 20 insertions(+), 14 deletions(-)
> 
> diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp
> index ed7ddb9..e2719fa 100644
> --- a/src/spice-streaming-agent.cpp
> +++ b/src/spice-streaming-agent.cpp
> @@ -348,17 +348,30 @@ do_capture(const char *streamport, FILE *f_log)
>      if (!capture)
>          throw std::runtime_error("cannot find a suitable capture system");
>  
> -    streamfd = open(streamport, O_RDWR);
> -    if (streamfd < 0)
> -        // TODO was syslog(LOG_ERR, "Failed to open %s: %s\n", streamport,
> strerror(errno));
> -        throw std::runtime_error("failed to open streaming device");
> +    struct Stream
> +    {
> +        Stream(const char *name, int &fd): fd(fd)
> +        {
> +            fd = open(name, O_RDWR);
> +            if (fd < 0)
> +                throw std::runtime_error("failed to open streaming device");
> +        }
> +        ~Stream()
> +        {
> +            if (fd >= 0)
> +                close(fd);
> +            fd = -1;
> +        }
> +        int &fd;
> +    }
> +    stream(streamport, streamfd);
>  
>      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 +422,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__);

I'm fine with it. This was intended as a preparatory patch for the
other commits.

> @@ -441,7 +448,7 @@ int main(int argc, char* argv[])
>      if (isatty(fileno(stderr)) && isatty(fileno(stdin))) {
>          stdin_ok = true;
>      }
> -
> +
>      openlog("spice-streaming-agent", stdin_ok? (LOG_PERROR|LOG_PID) :
>      LOG_PID, LOG_USER);
>      setlogmask(logmask);
>  
> @@ -526,4 +533,3 @@ int main(int argc, char* argv[])
>      closelog();
>      return ret;
>  }
> -

I would remove these spurious changes.

Frediano