> On 28 Feb 2018, at 17:36, Christophe Fergeau <cfergeau@xxxxxxxxxx> 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@xxxxxxxxxx>
>>
>> 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@xxxxxxxxxx>
>> ---
>> 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@xxxxxxxxxxxxxxxxxxxxx
>> https://lists.freedesktop.org/mailman/listinfo/spice-devel
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel