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

[Frediano Ziglio]

> 
> On Tue, 2018-02-20 at 20:48 +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 5613934..8d91f2d 100644
> > --- a/src/spice-streaming-agent.cpp
> > +++ b/src/spice-streaming-agent.cpp
> > @@ -77,10 +77,11 @@ static int have_something_to_read(int timeout)
> >      return 0;
> >  }
> >  
> > +static void handle_stream_start_stop(uint32_t len);
> > +
> 
> Why not put the method here instead of a forward declaration (and
> adding more in the future)?
> 

Easier to read if you read the file from top to bottom.

> >  static void read_command_from_device(void)
> >  {
> >      StreamDevHeader hdr;
> > -    uint8_t msg[64];
> >      int n;
> >  
> >      std::lock_guard<std::mutex> stream_guard(stream_mtx);
> > @@ -93,17 +94,26 @@ static void read_command_from_device(void)
> >          throw std::runtime_error("BAD VERSION " +
> >          std::to_string(hdr.protocol_version) +
> >                                   " (expected is " +
> >                                   std::to_string(STREAM_DEVICE_PROTOCOL) +
> >                                   ")");
> >      }
> > -    if (hdr.type != STREAM_TYPE_START_STOP) {
> > -        throw std::runtime_error("UNKNOWN msg of type " +
> > std::to_string(hdr.type));
> > +
> > +    switch (hdr.type) {
> > +    case STREAM_TYPE_START_STOP:
> > +        return handle_stream_start_stop(hdr.size);
> >      }
> > -    if (hdr.size >= sizeof(msg)) {
> > -        throw std::runtime_error("msg size (" + std::to_string(hdr.size) +
> > ") is too long "
> > +    throw std::runtime_error("UNKNOWN msg of type " +
> > std::to_string(hdr.type));
> > +}
> > +
> > +static void handle_stream_start_stop(uint32_t len)
> > +{
> > +    uint8_t msg[256];
> > +
> > +    if (len >= sizeof(msg)) {
> > +        throw std::runtime_error("msg size (" + std::to_string(len) + ")
> > is too long "
> >                                   "(longer than " +
> >                                   std::to_string(sizeof(msg)) + ")");
> >      }
> > -    n = read(streamfd, &msg, hdr.size);
> > -    if (n != hdr.size) {
> > +    int n = read(streamfd, &msg, len);
> > +    if (n != len) {
> >          throw std::runtime_error("read command from device FAILED -- read
> >          " + std::to_string(n) +
> > -                                 " expected " + std::to_string(hdr.size));
> > +                                 " expected " + std::to_string(len));
> >      }
> >      streaming_requested = (msg[0] != 0); /* num_codecs */
> >      syslog(LOG_INFO, "GOT START_STOP message -- request to %s
> >      streaming\n",
> 
> Apart from the forward declaration,
> 
> Acked-by: Lukáš Hrázký <lhrazky at redhat.com>
> 

Frediano