[Spice-devel] [PATCH 08/14] Use RAII to cleanup stream in case of exception or return

Frediano Ziglio fziglio at redhat.com
Thu Feb 15 10:04:02 UTC 2018


> 
> 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>

I honestly prefer the "defer" style way.

Beside that you correctly pointed out that there's a race condition
on cursor thread which could lead the cursor thread to attempt writing
the cursor shape before the device is open and you proposed some fixes
in a different series.
I think would be a better fix for this.

> ---
>  src/spice-streaming-agent.cpp | 32 ++++++++++++++++++++------------
>  1 file changed, 20 insertions(+), 12 deletions(-)
> 
> diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp
> index 25a38a6..e9ef310 100644
> --- a/src/spice-streaming-agent.cpp
> +++ b/src/spice-streaming-agent.cpp
> @@ -53,6 +53,23 @@ struct SpiceStreamDataMessage
>      StreamMsgData msg;
>  };
>  
> +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;
> +};
> +
>  static bool streaming_requested = false;
>  static bool quit_requested = false;
>  static bool log_binary = false;
> @@ -354,17 +371,14 @@ static void cursor_changes(Display *display, int
> event_base)
>  static void
>  do_capture(const char *streamport, FILE *f_log)
>  {
> -    streamfd = open(streamport.c_str(), O_RDWR);
> -    if (streamfd < 0)
> -        throw std::runtime_error("failed to open the streaming device (" +
> -                                 streamport + "): " + strerror(errno));
> +    Stream stream(streamport, streamfd);
>  
>      unsigned int frame_count = 0;
>      while (!quit_requested) {
>          while (!quit_requested && !streaming_requested) {
>              if (read_command(true) < 0) {
>                  syslog(LOG_ERR, "FAILED to read command\n");
> -                goto done;
> +                return;
>              }
>          }
>  
> @@ -422,16 +436,10 @@ do_capture(const char *streamport, FILE *f_log)
>              //usleep(1);
>              if (read_command(false) < 0) {
>                  syslog(LOG_ERR, "FAILED to read command\n");
> -                goto done;
> +                return;
>              }
>          }
>      }
> -
> -done:
> -    if (streamfd >= 0) {
> -        close(streamfd);
> -        streamfd = -1;
> -    }
>  }
>  
>  #define arg_error(...) syslog(LOG_ERR, ## __VA_ARGS__);

Frediano


More information about the Spice-devel mailing list