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

[Christophe de Dinechin]

> On 10 Nov 2017, at 10:38, Frediano Ziglio <fziglio at redhat.com> wrote:
> 
>> 
>> 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.

My emacs is configured to remove trailing spaces and empty lines at end of file automatically. Line 444 of the master I was working on (a61a00f12e1f8e1b982441bb4571376e3b2262e1) has trailing spaces, and there is an empty line at end of file. Can we agree that such cleanup is OK?

> 
> Frediano
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel