[06/22] Get rid of C-style memset initializations, use C++ style aggregates

Submitted by Christophe de Dinechin on Feb. 28, 2018, 3:43 p.m.

Details

Message ID 20180228154325.25791-7-christophe@dinechin.org
State New
Headers show
Series "streaming-agent: C++ refactoring" ( rev: 1 ) in Spice

Not browsing as part of any series.

Commit Message

Christophe de Dinechin Feb. 28, 2018, 3:43 p.m.
From: Christophe de Dinechin <dinechin@redhat.com>

C++-style aggregate initialization with explicit field names is more
readable and requires less repeated typing.

Like zero initialization, aggregate initialization preserves C++ type
safety.  However, unlike zero initialization, it does not guarantee
that padding is properly initialized. This is not a real problem in
our code where all padding is explicit, but it it worth keeping in
mind.

Also note that GCC is a bit picky and will generate strange
"unsupported" or "not implemented" errors if the fields are not all
initialized in exact declaration order. It is useful, however, to get
an error if a field is added and not initialized.

Writing this patch also highlighted an inconsistency between the type
of a 'codec' local variable and the type used in the actual data
type. Changed the type of the variable to uint8_t to match that in the
structure declaration.

Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
---
 src/spice-streaming-agent.cpp | 47 ++++++++++++++++++++++++++-----------------
 1 file changed, 28 insertions(+), 19 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp
index acae939..7b56458 100644
--- a/src/spice-streaming-agent.cpp
+++ b/src/spice-streaming-agent.cpp
@@ -221,19 +221,24 @@  write_all(int fd, const void *buf, const size_t len)
     return written;
 }
 
-static int spice_stream_send_format(int streamfd, unsigned w, unsigned h, unsigned c)
+static int spice_stream_send_format(int streamfd, unsigned w, unsigned h, uint8_t c)
 {
-
-    SpiceStreamFormatMessage msg;
-    const size_t msgsize = sizeof(msg);
-    const size_t hdrsize  = sizeof(msg.hdr);
-    memset(&msg, 0, msgsize);
-    msg.hdr.protocol_version = STREAM_DEVICE_PROTOCOL;
-    msg.hdr.type = STREAM_TYPE_FORMAT;
-    msg.hdr.size = msgsize - hdrsize; /* includes only the body? */
-    msg.msg.width = w;
-    msg.msg.height = h;
-    msg.msg.codec = c;
+    const size_t msgsize = sizeof(SpiceStreamFormatMessage);
+    const size_t hdrsize  = sizeof(StreamDevHeader);
+    SpiceStreamFormatMessage msg = {
+        .hdr = {
+            .protocol_version = STREAM_DEVICE_PROTOCOL,
+            .padding = 0,       // Workaround GCC "not implemented" bug
+            .type = STREAM_TYPE_FORMAT,
+            .size = msgsize - hdrsize
+        },
+        .msg = {
+            .width = w,
+            .height = h,
+            .codec = c,
+            .padding1 = { }
+        }
+    };
     syslog(LOG_DEBUG, "writing format\n");
     std::lock_guard<std::mutex> stream_guard(stream_mtx);
     if (write_all(streamfd, &msg, msgsize) != msgsize) {
@@ -244,14 +249,18 @@  static int spice_stream_send_format(int streamfd, unsigned w, unsigned h, unsign
 
 static int spice_stream_send_frame(int streamfd, const void *buf, const unsigned size)
 {
-    SpiceStreamDataMessage msg;
-    const size_t msgsize = sizeof(msg);
     ssize_t n;
+    const size_t msgsize = sizeof(SpiceStreamFormatMessage);
+    SpiceStreamDataMessage msg = {
+        .hdr = {
+            .protocol_version = STREAM_DEVICE_PROTOCOL,
+            .padding = 0,       // Workaround GCC "not implemented" bug
+            .type = STREAM_TYPE_DATA,
+            .size = size  /* includes only the body? */
+        },
+        .msg = {}
+    };
 
-    memset(&msg, 0, msgsize);
-    msg.hdr.protocol_version = STREAM_DEVICE_PROTOCOL;
-    msg.hdr.type = STREAM_TYPE_DATA;
-    msg.hdr.size = size; /* includes only the body? */
     std::lock_guard<std::mutex> stream_guard(stream_mtx);
     n = write_all(streamfd, &msg, msgsize);
     syslog(LOG_DEBUG,
@@ -424,7 +433,7 @@  do_capture(int streamfd, const char *streamport, FILE *f_log)
 
             if (frame.stream_start) {
                 unsigned width, height;
-                unsigned char codec;
+                uint8_t codec;
 
                 width = frame.size.width;
                 height = frame.size.height;

Comments

My understanding is that the previous iteration was quite controversial,
I would just drop it from the series unless you get acks from everyone
involved this time.

Christophe

On Wed, Feb 28, 2018 at 04:43:09PM +0100, Christophe de Dinechin wrote:
> From: Christophe de Dinechin <dinechin@redhat.com>
> 
> C++-style aggregate initialization with explicit field names is more
> readable and requires less repeated typing.
> 
> Like zero initialization, aggregate initialization preserves C++ type
> safety.  However, unlike zero initialization, it does not guarantee
> that padding is properly initialized. This is not a real problem in
> our code where all padding is explicit, but it it worth keeping in
> mind.
> 
> Also note that GCC is a bit picky and will generate strange
> "unsupported" or "not implemented" errors if the fields are not all
> initialized in exact declaration order. It is useful, however, to get
> an error if a field is added and not initialized.
> 
> Writing this patch also highlighted an inconsistency between the type
> of a 'codec' local variable and the type used in the actual data
> type. Changed the type of the variable to uint8_t to match that in the
> structure declaration.
> 
> Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
> ---
>  src/spice-streaming-agent.cpp | 47 ++++++++++++++++++++++++++-----------------
>  1 file changed, 28 insertions(+), 19 deletions(-)
> 
> diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp
> index acae939..7b56458 100644
> --- a/src/spice-streaming-agent.cpp
> +++ b/src/spice-streaming-agent.cpp
> @@ -221,19 +221,24 @@ write_all(int fd, const void *buf, const size_t len)
>      return written;
>  }
>  
> -static int spice_stream_send_format(int streamfd, unsigned w, unsigned h, unsigned c)
> +static int spice_stream_send_format(int streamfd, unsigned w, unsigned h, uint8_t c)
>  {
> -
> -    SpiceStreamFormatMessage msg;
> -    const size_t msgsize = sizeof(msg);
> -    const size_t hdrsize  = sizeof(msg.hdr);
> -    memset(&msg, 0, msgsize);
> -    msg.hdr.protocol_version = STREAM_DEVICE_PROTOCOL;
> -    msg.hdr.type = STREAM_TYPE_FORMAT;
> -    msg.hdr.size = msgsize - hdrsize; /* includes only the body? */
> -    msg.msg.width = w;
> -    msg.msg.height = h;
> -    msg.msg.codec = c;
> +    const size_t msgsize = sizeof(SpiceStreamFormatMessage);
> +    const size_t hdrsize  = sizeof(StreamDevHeader);
> +    SpiceStreamFormatMessage msg = {
> +        .hdr = {
> +            .protocol_version = STREAM_DEVICE_PROTOCOL,
> +            .padding = 0,       // Workaround GCC "not implemented" bug
> +            .type = STREAM_TYPE_FORMAT,
> +            .size = msgsize - hdrsize
> +        },
> +        .msg = {
> +            .width = w,
> +            .height = h,
> +            .codec = c,
> +            .padding1 = { }
> +        }
> +    };
>      syslog(LOG_DEBUG, "writing format\n");
>      std::lock_guard<std::mutex> stream_guard(stream_mtx);
>      if (write_all(streamfd, &msg, msgsize) != msgsize) {
> @@ -244,14 +249,18 @@ static int spice_stream_send_format(int streamfd, unsigned w, unsigned h, unsign
>  
>  static int spice_stream_send_frame(int streamfd, const void *buf, const unsigned size)
>  {
> -    SpiceStreamDataMessage msg;
> -    const size_t msgsize = sizeof(msg);
>      ssize_t n;
> +    const size_t msgsize = sizeof(SpiceStreamFormatMessage);
> +    SpiceStreamDataMessage msg = {
> +        .hdr = {
> +            .protocol_version = STREAM_DEVICE_PROTOCOL,
> +            .padding = 0,       // Workaround GCC "not implemented" bug
> +            .type = STREAM_TYPE_DATA,
> +            .size = size  /* includes only the body? */
> +        },
> +        .msg = {}
> +    };
>  
> -    memset(&msg, 0, msgsize);
> -    msg.hdr.protocol_version = STREAM_DEVICE_PROTOCOL;
> -    msg.hdr.type = STREAM_TYPE_DATA;
> -    msg.hdr.size = size; /* includes only the body? */
>      std::lock_guard<std::mutex> stream_guard(stream_mtx);
>      n = write_all(streamfd, &msg, msgsize);
>      syslog(LOG_DEBUG,
> @@ -424,7 +433,7 @@ do_capture(int streamfd, const char *streamport, FILE *f_log)
>  
>              if (frame.stream_start) {
>                  unsigned width, height;
> -                unsigned char codec;
> +                uint8_t codec;
>  
>                  width = frame.size.width;
>                  height = frame.size.height;
> -- 
> 2.13.5 (Apple Git-94)
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
On Wed, 2018-02-28 at 17:36 +0100, Christophe Fergeau wrote:
> My understanding is that the previous iteration was quite controversial,
> I would just drop it from the series unless you get acks from everyone
> involved this time.

I've only commented on compiler support, it seems it's fine, so no
problem for me. I'll leave the ack to Frediano, he understands better
the padding issues.

> Christophe
> 
> On Wed, Feb 28, 2018 at 04:43:09PM +0100, Christophe de Dinechin wrote:
> > From: Christophe de Dinechin <dinechin@redhat.com>
> > 
> > C++-style aggregate initialization with explicit field names is more
> > readable and requires less repeated typing.
> > 
> > Like zero initialization, aggregate initialization preserves C++ type
> > safety.  However, unlike zero initialization, it does not guarantee
> > that padding is properly initialized. This is not a real problem in
> > our code where all padding is explicit, but it it worth keeping in

"it is".

> > mind.
> > 
> > Also note that GCC is a bit picky and will generate strange
> > "unsupported" or "not implemented" errors if the fields are not all
> > initialized in exact declaration order. It is useful, however, to get
> > an error if a field is added and not initialized.
> > 
> > Writing this patch also highlighted an inconsistency between the type
> > of a 'codec' local variable and the type used in the actual data
> > type. Changed the type of the variable to uint8_t to match that in the
> > structure declaration.
> > 
> > Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
> > ---
> >  src/spice-streaming-agent.cpp | 47 ++++++++++++++++++++++++++-----------------
> >  1 file changed, 28 insertions(+), 19 deletions(-)
> > 
> > diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp
> > index acae939..7b56458 100644
> > --- a/src/spice-streaming-agent.cpp
> > +++ b/src/spice-streaming-agent.cpp
> > @@ -221,19 +221,24 @@ write_all(int fd, const void *buf, const size_t len)
> >      return written;
> >  }
> >  
> > -static int spice_stream_send_format(int streamfd, unsigned w, unsigned h, unsigned c)
> > +static int spice_stream_send_format(int streamfd, unsigned w, unsigned h, uint8_t c)
> >  {
> > -
> > -    SpiceStreamFormatMessage msg;
> > -    const size_t msgsize = sizeof(msg);
> > -    const size_t hdrsize  = sizeof(msg.hdr);
> > -    memset(&msg, 0, msgsize);
> > -    msg.hdr.protocol_version = STREAM_DEVICE_PROTOCOL;
> > -    msg.hdr.type = STREAM_TYPE_FORMAT;
> > -    msg.hdr.size = msgsize - hdrsize; /* includes only the body? */
> > -    msg.msg.width = w;
> > -    msg.msg.height = h;
> > -    msg.msg.codec = c;
> > +    const size_t msgsize = sizeof(SpiceStreamFormatMessage);
> > +    const size_t hdrsize  = sizeof(StreamDevHeader);
> > +    SpiceStreamFormatMessage msg = {
> > +        .hdr = {
> > +            .protocol_version = STREAM_DEVICE_PROTOCOL,
> > +            .padding = 0,       // Workaround GCC "not implemented" bug
> > +            .type = STREAM_TYPE_FORMAT,
> > +            .size = msgsize - hdrsize
> > +        },
> > +        .msg = {
> > +            .width = w,
> > +            .height = h,
> > +            .codec = c,
> > +            .padding1 = { }
> > +        }
> > +    };
> >      syslog(LOG_DEBUG, "writing format\n");
> >      std::lock_guard<std::mutex> stream_guard(stream_mtx);
> >      if (write_all(streamfd, &msg, msgsize) != msgsize) {
> > @@ -244,14 +249,18 @@ static int spice_stream_send_format(int streamfd, unsigned w, unsigned h, unsign
> >  
> >  static int spice_stream_send_frame(int streamfd, const void *buf, const unsigned size)
> >  {
> > -    SpiceStreamDataMessage msg;
> > -    const size_t msgsize = sizeof(msg);
> >      ssize_t n;
> > +    const size_t msgsize = sizeof(SpiceStreamFormatMessage);
> > +    SpiceStreamDataMessage msg = {
> > +        .hdr = {
> > +            .protocol_version = STREAM_DEVICE_PROTOCOL,
> > +            .padding = 0,       // Workaround GCC "not implemented" bug
> > +            .type = STREAM_TYPE_DATA,
> > +            .size = size  /* includes only the body? */

The question in the comment? :)

> > +        },
> > +        .msg = {}
> > +    };
> >  
> > -    memset(&msg, 0, msgsize);
> > -    msg.hdr.protocol_version = STREAM_DEVICE_PROTOCOL;
> > -    msg.hdr.type = STREAM_TYPE_DATA;
> > -    msg.hdr.size = size; /* includes only the body? */
> >      std::lock_guard<std::mutex> stream_guard(stream_mtx);
> >      n = write_all(streamfd, &msg, msgsize);
> >      syslog(LOG_DEBUG,
> > @@ -424,7 +433,7 @@ do_capture(int streamfd, const char *streamport, FILE *f_log)
> >  
> >              if (frame.stream_start) {
> >                  unsigned width, height;
> > -                unsigned char codec;
> > +                uint8_t codec;
> >  
> >                  width = frame.size.width;
> >                  height = frame.size.height;
> > -- 
> > 2.13.5 (Apple Git-94)
> > 
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
> On 28 Feb 2018, at 17:36, Christophe Fergeau <cfergeau@redhat.com> wrote:
> 
> My understanding is that the previous iteration was quite controversial,
> I would just drop it from the series unless you get acks from everyone
> involved this time.

It’s a bit difficult to drop that from the series, as it is a core element of the next steps if you look carefully.


> 
> Christophe
> 
> On Wed, Feb 28, 2018 at 04:43:09PM +0100, Christophe de Dinechin wrote:
>> From: Christophe de Dinechin <dinechin@redhat.com>
>> 
>> C++-style aggregate initialization with explicit field names is more
>> readable and requires less repeated typing.
>> 
>> Like zero initialization, aggregate initialization preserves C++ type
>> safety.  However, unlike zero initialization, it does not guarantee
>> that padding is properly initialized. This is not a real problem in
>> our code where all padding is explicit, but it it worth keeping in
>> mind.
>> 
>> Also note that GCC is a bit picky and will generate strange
>> "unsupported" or "not implemented" errors if the fields are not all
>> initialized in exact declaration order. It is useful, however, to get
>> an error if a field is added and not initialized.
>> 
>> Writing this patch also highlighted an inconsistency between the type
>> of a 'codec' local variable and the type used in the actual data
>> type. Changed the type of the variable to uint8_t to match that in the
>> structure declaration.
>> 
>> Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
>> ---
>> src/spice-streaming-agent.cpp | 47 ++++++++++++++++++++++++++-----------------
>> 1 file changed, 28 insertions(+), 19 deletions(-)
>> 
>> diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp
>> index acae939..7b56458 100644
>> --- a/src/spice-streaming-agent.cpp
>> +++ b/src/spice-streaming-agent.cpp
>> @@ -221,19 +221,24 @@ write_all(int fd, const void *buf, const size_t len)
>>     return written;
>> }
>> 
>> -static int spice_stream_send_format(int streamfd, unsigned w, unsigned h, unsigned c)
>> +static int spice_stream_send_format(int streamfd, unsigned w, unsigned h, uint8_t c)
>> {
>> -
>> -    SpiceStreamFormatMessage msg;
>> -    const size_t msgsize = sizeof(msg);
>> -    const size_t hdrsize  = sizeof(msg.hdr);
>> -    memset(&msg, 0, msgsize);
>> -    msg.hdr.protocol_version = STREAM_DEVICE_PROTOCOL;
>> -    msg.hdr.type = STREAM_TYPE_FORMAT;
>> -    msg.hdr.size = msgsize - hdrsize; /* includes only the body? */
>> -    msg.msg.width = w;
>> -    msg.msg.height = h;
>> -    msg.msg.codec = c;
>> +    const size_t msgsize = sizeof(SpiceStreamFormatMessage);
>> +    const size_t hdrsize  = sizeof(StreamDevHeader);
>> +    SpiceStreamFormatMessage msg = {
>> +        .hdr = {
>> +            .protocol_version = STREAM_DEVICE_PROTOCOL,
>> +            .padding = 0,       // Workaround GCC "not implemented" bug
>> +            .type = STREAM_TYPE_FORMAT,
>> +            .size = msgsize - hdrsize
>> +        },
>> +        .msg = {
>> +            .width = w,
>> +            .height = h,
>> +            .codec = c,
>> +            .padding1 = { }
>> +        }
>> +    };
>>     syslog(LOG_DEBUG, "writing format\n");
>>     std::lock_guard<std::mutex> stream_guard(stream_mtx);
>>     if (write_all(streamfd, &msg, msgsize) != msgsize) {
>> @@ -244,14 +249,18 @@ static int spice_stream_send_format(int streamfd, unsigned w, unsigned h, unsign
>> 
>> static int spice_stream_send_frame(int streamfd, const void *buf, const unsigned size)
>> {
>> -    SpiceStreamDataMessage msg;
>> -    const size_t msgsize = sizeof(msg);
>>     ssize_t n;
>> +    const size_t msgsize = sizeof(SpiceStreamFormatMessage);
>> +    SpiceStreamDataMessage msg = {
>> +        .hdr = {
>> +            .protocol_version = STREAM_DEVICE_PROTOCOL,
>> +            .padding = 0,       // Workaround GCC "not implemented" bug
>> +            .type = STREAM_TYPE_DATA,
>> +            .size = size  /* includes only the body? */
>> +        },
>> +        .msg = {}
>> +    };
>> 
>> -    memset(&msg, 0, msgsize);
>> -    msg.hdr.protocol_version = STREAM_DEVICE_PROTOCOL;
>> -    msg.hdr.type = STREAM_TYPE_DATA;
>> -    msg.hdr.size = size; /* includes only the body? */
>>     std::lock_guard<std::mutex> stream_guard(stream_mtx);
>>     n = write_all(streamfd, &msg, msgsize);
>>     syslog(LOG_DEBUG,
>> @@ -424,7 +433,7 @@ do_capture(int streamfd, const char *streamport, FILE *f_log)
>> 
>>             if (frame.stream_start) {
>>                 unsigned width, height;
>> -                unsigned char codec;
>> +                uint8_t codec;
>> 
>>                 width = frame.size.width;
>>                 height = frame.size.height;
>> -- 
>> 2.13.5 (Apple Git-94)
>> 
>> _______________________________________________
>> Spice-devel mailing list
>> Spice-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/spice-devel
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
> On 1 Mar 2018, at 10:51, Lukáš Hrázký <lhrazky@redhat.com> wrote:
> 
> On Wed, 2018-02-28 at 17:36 +0100, Christophe Fergeau wrote:
>> My understanding is that the previous iteration was quite controversial,
>> I would just drop it from the series unless you get acks from everyone
>> involved this time.
> 
> I've only commented on compiler support, it seems it's fine, so no
> problem for me. I'll leave the ack to Frediano, he understands better
> the padding issues.
> 
>> Christophe
>> 
>> On Wed, Feb 28, 2018 at 04:43:09PM +0100, Christophe de Dinechin wrote:
>>> From: Christophe de Dinechin <dinechin@redhat.com>
>>> 
>>> C++-style aggregate initialization with explicit field names is more
>>> readable and requires less repeated typing.
>>> 
>>> Like zero initialization, aggregate initialization preserves C++ type
>>> safety.  However, unlike zero initialization, it does not guarantee
>>> that padding is properly initialized. This is not a real problem in
>>> our code where all padding is explicit, but it it worth keeping in
> 
> "it is”.

Fixed

> 
>>> mind.
>>> 
>>> Also note that GCC is a bit picky and will generate strange
>>> "unsupported" or "not implemented" errors if the fields are not all
>>> initialized in exact declaration order. It is useful, however, to get
>>> an error if a field is added and not initialized.
>>> 
>>> Writing this patch also highlighted an inconsistency between the type
>>> of a 'codec' local variable and the type used in the actual data
>>> type. Changed the type of the variable to uint8_t to match that in the
>>> structure declaration.
>>> 
>>> Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
>>> ---
>>> src/spice-streaming-agent.cpp | 47 ++++++++++++++++++++++++++-----------------
>>> 1 file changed, 28 insertions(+), 19 deletions(-)
>>> 
>>> diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp
>>> index acae939..7b56458 100644
>>> --- a/src/spice-streaming-agent.cpp
>>> +++ b/src/spice-streaming-agent.cpp
>>> @@ -221,19 +221,24 @@ write_all(int fd, const void *buf, const size_t len)
>>>     return written;
>>> }
>>> 
>>> -static int spice_stream_send_format(int streamfd, unsigned w, unsigned h, unsigned c)
>>> +static int spice_stream_send_format(int streamfd, unsigned w, unsigned h, uint8_t c)
>>> {
>>> -
>>> -    SpiceStreamFormatMessage msg;
>>> -    const size_t msgsize = sizeof(msg);
>>> -    const size_t hdrsize  = sizeof(msg.hdr);
>>> -    memset(&msg, 0, msgsize);
>>> -    msg.hdr.protocol_version = STREAM_DEVICE_PROTOCOL;
>>> -    msg.hdr.type = STREAM_TYPE_FORMAT;
>>> -    msg.hdr.size = msgsize - hdrsize; /* includes only the body? */
>>> -    msg.msg.width = w;
>>> -    msg.msg.height = h;
>>> -    msg.msg.codec = c;
>>> +    const size_t msgsize = sizeof(SpiceStreamFormatMessage);
>>> +    const size_t hdrsize  = sizeof(StreamDevHeader);
>>> +    SpiceStreamFormatMessage msg = {
>>> +        .hdr = {
>>> +            .protocol_version = STREAM_DEVICE_PROTOCOL,
>>> +            .padding = 0,       // Workaround GCC "not implemented" bug
>>> +            .type = STREAM_TYPE_FORMAT,
>>> +            .size = msgsize - hdrsize
>>> +        },
>>> +        .msg = {
>>> +            .width = w,
>>> +            .height = h,
>>> +            .codec = c,
>>> +            .padding1 = { }
>>> +        }
>>> +    };
>>>     syslog(LOG_DEBUG, "writing format\n");
>>>     std::lock_guard<std::mutex> stream_guard(stream_mtx);
>>>     if (write_all(streamfd, &msg, msgsize) != msgsize) {
>>> @@ -244,14 +249,18 @@ static int spice_stream_send_format(int streamfd, unsigned w, unsigned h, unsign
>>> 
>>> static int spice_stream_send_frame(int streamfd, const void *buf, const unsigned size)
>>> {
>>> -    SpiceStreamDataMessage msg;
>>> -    const size_t msgsize = sizeof(msg);
>>>     ssize_t n;
>>> +    const size_t msgsize = sizeof(SpiceStreamFormatMessage);
>>> +    SpiceStreamDataMessage msg = {
>>> +        .hdr = {
>>> +            .protocol_version = STREAM_DEVICE_PROTOCOL,
>>> +            .padding = 0,       // Workaround GCC "not implemented" bug
>>> +            .type = STREAM_TYPE_DATA,
>>> +            .size = size  /* includes only the body? */
> 
> The question in the comment? :)

It was in the original. Not sure why.

> 
>>> +        },
>>> +        .msg = {}
>>> +    };
>>> 
>>> -    memset(&msg, 0, msgsize);
>>> -    msg.hdr.protocol_version = STREAM_DEVICE_PROTOCOL;
>>> -    msg.hdr.type = STREAM_TYPE_DATA;
>>> -    msg.hdr.size = size; /* includes only the body? */
>>>     std::lock_guard<std::mutex> stream_guard(stream_mtx);
>>>     n = write_all(streamfd, &msg, msgsize);
>>>     syslog(LOG_DEBUG,
>>> @@ -424,7 +433,7 @@ do_capture(int streamfd, const char *streamport, FILE *f_log)
>>> 
>>>             if (frame.stream_start) {
>>>                 unsigned width, height;
>>> -                unsigned char codec;
>>> +                uint8_t codec;
>>> 
>>>                 width = frame.size.width;
>>>                 height = frame.size.height;
>>> -- 
>>> 2.13.5 (Apple Git-94)
>>> 
>>> _______________________________________________
>>> Spice-devel mailing list
>>> Spice-devel@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/spice-devel
>> 
>> _______________________________________________
>> Spice-devel mailing list
>> Spice-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/spice-devel
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel