[Spice-devel] [PATCH spice-streaming-agent 4/6] Separate handling start/stop message from server

[Lukáš Hrázký]

On Mon, 2018-02-19 at 15:52 +0000, Frediano Ziglio wrote:
> Prepare to add support for other messages.
> 
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
>  src/spice-streaming-agent.cpp | 26 ++++++++++++++++++--------
>  1 file changed, 18 insertions(+), 8 deletions(-)
> 
> diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp
> index 08f4b48..41b4d3d 100644
> --- a/src/spice-streaming-agent.cpp
> +++ b/src/spice-streaming-agent.cpp
> @@ -78,10 +78,11 @@ static int have_something_to_read(int timeout)
>      return 0;
>  }
>  
> +static int read_stream_start_stop(uint32_t len);
> +
>  static int read_command_from_device(void)
>  {
>      StreamDevHeader hdr;
> -    uint8_t msg[64];
>      int n;
>  
>      std::lock_guard<std::mutex> stream_guard(stream_mtx);
> @@ -93,15 +94,24 @@ static int read_command_from_device(void)
>          ERROR("BAD VERSION " << hdr.protocol_version <<
>                " (expected is " << STREAM_DEVICE_PROTOCOL << ")");
>      }
> -    if (hdr.type != STREAM_TYPE_START_STOP) {
> -        ERROR("UNKNOWN msg of type " << hdr.type);
> +
> +    switch (hdr.type) {
> +    case STREAM_TYPE_START_STOP:
> +        return read_stream_start_stop(hdr.size);
>      }
> -    if (hdr.size >= sizeof(msg)) {
> -        ERROR("msg size (" << hdr.size << ") is too long (longer than " << sizeof(msg));
> +    ERROR("UNKNOWN msg of type " << hdr.type);
> +}
> +
> +static int read_stream_start_stop(uint32_t len)
> +{
> +    uint8_t msg[256];
> +
> +    if (len >= sizeof(msg)) {
> +        ERROR("msg size (" << len << ") is too long (longer than " << sizeof(msg));
>      }
> -    n = read(streamfd, &msg, hdr.size);
> -    if (n != hdr.size) {
> -        ERROR("read command from device FAILED -- read " << n << " expected " << hdr.size);
> +    int n = read(streamfd, &msg, len);
> +    if (n != len) {
> +        ERROR("read command from device FAILED -- read " << n << " expected " << len);
>      }
>      streaming_requested = (msg[0] != 0); /* num_codecs */
>      syslog(LOG_INFO, "GOT START_STOP message -- request to %s streaming\n",

So, you started working on the same code as well :P I actually have
this part rewritten in a C++ way. I was surprised to not find it in
Christophe's patches. My head starts going crazy from all the different
patches to the same code...

I would suggest leaving this off until we switch to an encapsulated C++
implementation?

Lukas