Re: [Spice-devel] [PATCH spice-streaming-agent v3 4/4] Stub to handle errors from server

[Lukáš Hrázký]

On Wed, 2018-02-21 at 05:51 -0500, Frediano Ziglio wrote:
> > 
> > On Tue, 2018-02-20 at 20:48 +, Frediano Ziglio wrote:
> > > Base error message handling.
> > > 
> > > Signed-off-by: Frediano Ziglio 
> > > ---
> > >  src/spice-streaming-agent.cpp | 9 +
> > >  1 file changed, 9 insertions(+)
> > > 
> > > diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp
> > > index 31c655c..343b252 100644
> > > --- a/src/spice-streaming-agent.cpp
> > > +++ b/src/spice-streaming-agent.cpp
> > > @@ -81,6 +81,7 @@ static int have_something_to_read(int timeout)
> > >  
> > >  static void handle_stream_capabilities(uint32_t len);
> > >  static void handle_stream_start_stop(uint32_t len);
> > > +static void handle_stream_error(uint32_t len);
> > >  
> > >  static void read_command_from_device(void)
> > >  {
> > > @@ -101,6 +102,8 @@ static void read_command_from_device(void)
> > >  switch (hdr.type) {
> > >  case STREAM_TYPE_CAPABILITIES:
> > >  return handle_stream_capabilities(hdr.size);
> > > +case STREAM_TYPE_NOTIFY_ERROR:
> > > +return handle_stream_error(hdr.size);
> > >  case STREAM_TYPE_START_STOP:
> > >  return handle_stream_start_stop(hdr.size);
> > >  }
> > > @@ -154,6 +157,12 @@ static void handle_stream_capabilities(uint32_t len)
> > >  }
> > >  }
> > >  
> > > +static void handle_stream_error(uint32_t len)
> > > +{
> > > +// TODO read message and use it
> > > +throw std::runtime_error("got an error message from server");
> > > +}
> > > +
> > >  static int read_command(bool blocking)
> > >  {
> > >  int timeout = blocking?-1:0;
> > 
> > You didn't have to make it that explicit for me :) But thanks... I
> > don't find this commit necessary, it doesn't change the behavior and
> > kind of just adds noise to the history...?
> > 
> 
> I supposed can be a
> 
> // TODO: case STREAM_TYPE_NOTIFY_ERROR
> 
> (however you get a not much accurate message) or a
> 
>case STREAM_TYPE_NOTIFY_ERROR:
>   // TODO read message and use it
>   throw std::runtime_error("got an error message from server");
> 
> but not much expensive to have a stub either.
> 
> I checked all required messages are handled and I put in the switch,
> I think is a good way to avoid forgetting to implement some.
> 
> Frediano

If you want it in, sure... I just don't find it very useful.

Acked-by: Lukáš Hrázký 
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-streaming-agent v3 4/4] Stub to handle errors from server

[Frediano Ziglio]

> 
> On Tue, 2018-02-20 at 20:48 +, Frediano Ziglio wrote:
> > Base error message handling.
> > 
> > Signed-off-by: Frediano Ziglio 
> > ---
> >  src/spice-streaming-agent.cpp | 9 +
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp
> > index 31c655c..343b252 100644
> > --- a/src/spice-streaming-agent.cpp
> > +++ b/src/spice-streaming-agent.cpp
> > @@ -81,6 +81,7 @@ static int have_something_to_read(int timeout)
> >  
> >  static void handle_stream_capabilities(uint32_t len);
> >  static void handle_stream_start_stop(uint32_t len);
> > +static void handle_stream_error(uint32_t len);
> >  
> >  static void read_command_from_device(void)
> >  {
> > @@ -101,6 +102,8 @@ static void read_command_from_device(void)
> >  switch (hdr.type) {
> >  case STREAM_TYPE_CAPABILITIES:
> >  return handle_stream_capabilities(hdr.size);
> > +case STREAM_TYPE_NOTIFY_ERROR:
> > +return handle_stream_error(hdr.size);
> >  case STREAM_TYPE_START_STOP:
> >  return handle_stream_start_stop(hdr.size);
> >  }
> > @@ -154,6 +157,12 @@ static void handle_stream_capabilities(uint32_t len)
> >  }
> >  }
> >  
> > +static void handle_stream_error(uint32_t len)
> > +{
> > +// TODO read message and use it
> > +throw std::runtime_error("got an error message from server");
> > +}
> > +
> >  static int read_command(bool blocking)
> >  {
> >  int timeout = blocking?-1:0;
> 
> You didn't have to make it that explicit for me :) But thanks... I
> don't find this commit necessary, it doesn't change the behavior and
> kind of just adds noise to the history...?
> 

I supposed can be a

// TODO: case STREAM_TYPE_NOTIFY_ERROR

(however you get a not much accurate message) or a

   case STREAM_TYPE_NOTIFY_ERROR:
  // TODO read message and use it
  throw std::runtime_error("got an error message from server");

but not much expensive to have a stub either.

I checked all required messages are handled and I put in the switch,
I think is a good way to avoid forgetting to implement some.

Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-streaming-agent v3 4/4] Stub to handle errors from server

[Lukáš Hrázký]

On Tue, 2018-02-20 at 20:48 +, Frediano Ziglio wrote:
> Base error message handling.
> 
> Signed-off-by: Frediano Ziglio 
> ---
>  src/spice-streaming-agent.cpp | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp
> index 31c655c..343b252 100644
> --- a/src/spice-streaming-agent.cpp
> +++ b/src/spice-streaming-agent.cpp
> @@ -81,6 +81,7 @@ static int have_something_to_read(int timeout)
>  
>  static void handle_stream_capabilities(uint32_t len);
>  static void handle_stream_start_stop(uint32_t len);
> +static void handle_stream_error(uint32_t len);
>  
>  static void read_command_from_device(void)
>  {
> @@ -101,6 +102,8 @@ static void read_command_from_device(void)
>  switch (hdr.type) {
>  case STREAM_TYPE_CAPABILITIES:
>  return handle_stream_capabilities(hdr.size);
> +case STREAM_TYPE_NOTIFY_ERROR:
> +return handle_stream_error(hdr.size);
>  case STREAM_TYPE_START_STOP:
>  return handle_stream_start_stop(hdr.size);
>  }
> @@ -154,6 +157,12 @@ static void handle_stream_capabilities(uint32_t len)
>  }
>  }
>  
> +static void handle_stream_error(uint32_t len)
> +{
> +// TODO read message and use it
> +throw std::runtime_error("got an error message from server");
> +}
> +
>  static int read_command(bool blocking)
>  {
>  int timeout = blocking?-1:0;

You didn't have to make it that explicit for me :) But thanks... I
don't find this commit necessary, it doesn't change the behavior and
kind of just adds noise to the history...?
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH spice-streaming-agent v3 4/4] Stub to handle errors from server

[Frediano Ziglio]

Base error message handling.

Signed-off-by: Frediano Ziglio 
---
 src/spice-streaming-agent.cpp | 9 +
 1 file changed, 9 insertions(+)

diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp
index 31c655c..343b252 100644
--- a/src/spice-streaming-agent.cpp
+++ b/src/spice-streaming-agent.cpp
@@ -81,6 +81,7 @@ static int have_something_to_read(int timeout)
 
 static void handle_stream_capabilities(uint32_t len);
 static void handle_stream_start_stop(uint32_t len);
+static void handle_stream_error(uint32_t len);
 
 static void read_command_from_device(void)
 {
@@ -101,6 +102,8 @@ static void read_command_from_device(void)
 switch (hdr.type) {
 case STREAM_TYPE_CAPABILITIES:
 return handle_stream_capabilities(hdr.size);
+case STREAM_TYPE_NOTIFY_ERROR:
+return handle_stream_error(hdr.size);
 case STREAM_TYPE_START_STOP:
 return handle_stream_start_stop(hdr.size);
 }
@@ -154,6 +157,12 @@ static void handle_stream_capabilities(uint32_t len)
 }
 }
 
+static void handle_stream_error(uint32_t len)
+{
+// TODO read message and use it
+throw std::runtime_error("got an error message from server");
+}
+
 static int read_command(bool blocking)
 {
 int timeout = blocking?-1:0;
-- 
2.14.3

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel