Re: [Spice-devel] [PATCH spice-streaming-agent 1/6] Be more restrictive about error handling

[Uri Lublin]

On 02/19/2018 05:52 PM, Frediano Ziglio wrote:

There's no much point in ignoring the error if these errors cause the
communication to be out of sync.
Ignoring them just change the state to indetermidate.


Hi Frediano,

There is a typo in the last word.

I would be more specific, mentioning those are header errors.

(Currently, upon returning -1 in those cases, spice-streaming-agent
 exits).



Signed-off-by: Frediano Ziglio 


Acked-by: Uri Lublin 


---
  src/spice-streaming-agent.cpp | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp
index 45a92b9..1f41a6f 100644
--- a/src/spice-streaming-agent.cpp
+++ b/src/spice-streaming-agent.cpp
@@ -94,17 +94,17 @@ static int read_command_from_device(void)
  if (hdr.protocol_version != STREAM_DEVICE_PROTOCOL) {
  syslog(LOG_WARNING, "BAD VERSION %d (expected is %d)\n", 
hdr.protocol_version,
 STREAM_DEVICE_PROTOCOL);
-return 0; // return -1; -- fail over this ?
+return -1;
  }
  if (hdr.type != STREAM_TYPE_START_STOP) {
  syslog(LOG_WARNING, "UNKNOWN msg of type %d\n", hdr.type);
-return 0; // return -1;
+return -1;
  }
  if (hdr.size >= sizeof(msg)) {
  syslog(LOG_WARNING,
 "msg size (%d) is too long (longer than %lu)\n",
 hdr.size, sizeof(msg));
-return 0; // return -1;
+return -1;
  }
  n = read(streamfd, , hdr.size);
  if (n != hdr.size) {



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

Re: [Spice-devel] [PATCH spice-streaming-agent 1/6] Be more restrictive about error handling

[Frediano Ziglio]

> 
> On Mon, 2018-02-19 at 15:52 +, Frediano Ziglio wrote:
> > There's no much point in ignoring the error if these errors cause the
> > communication to be out of sync.
> > Ignoring them just change the state to indetermidate.
> > 
> > Signed-off-by: Frediano Ziglio 
> > ---
> >  src/spice-streaming-agent.cpp | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp
> > index 45a92b9..1f41a6f 100644
> > --- a/src/spice-streaming-agent.cpp
> > +++ b/src/spice-streaming-agent.cpp
> > @@ -94,17 +94,17 @@ static int read_command_from_device(void)
> >  if (hdr.protocol_version != STREAM_DEVICE_PROTOCOL) {
> >  syslog(LOG_WARNING, "BAD VERSION %d (expected is %d)\n",
> >  hdr.protocol_version,
> > STREAM_DEVICE_PROTOCOL);
> > -return 0; // return -1; -- fail over this ?
> > +return -1;
> >  }
> >  if (hdr.type != STREAM_TYPE_START_STOP) {
> >  syslog(LOG_WARNING, "UNKNOWN msg of type %d\n", hdr.type);
> > -return 0; // return -1;
> > +return -1;
> >  }
> >  if (hdr.size >= sizeof(msg)) {
> >  syslog(LOG_WARNING,
> > "msg size (%d) is too long (longer than %lu)\n",
> > hdr.size, sizeof(msg));
> > -return 0; // return -1;
> > +return -1;
> >  }
> >  n = read(streamfd, , hdr.size);
> >  if (n != hdr.size) {
> 
> Hi, why not, but why not skip this and use the exceptions straight
> away?
> 
> Lukas
> 

Seems more incremental that way.

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 1/6] Be more restrictive about error handling

[Lukáš Hrázký]

On Mon, 2018-02-19 at 15:52 +, Frediano Ziglio wrote:
> There's no much point in ignoring the error if these errors cause the
> communication to be out of sync.
> Ignoring them just change the state to indetermidate.
> 
> Signed-off-by: Frediano Ziglio 
> ---
>  src/spice-streaming-agent.cpp | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp
> index 45a92b9..1f41a6f 100644
> --- a/src/spice-streaming-agent.cpp
> +++ b/src/spice-streaming-agent.cpp
> @@ -94,17 +94,17 @@ static int read_command_from_device(void)
>  if (hdr.protocol_version != STREAM_DEVICE_PROTOCOL) {
>  syslog(LOG_WARNING, "BAD VERSION %d (expected is %d)\n", 
> hdr.protocol_version,
> STREAM_DEVICE_PROTOCOL);
> -return 0; // return -1; -- fail over this ?
> +return -1;
>  }
>  if (hdr.type != STREAM_TYPE_START_STOP) {
>  syslog(LOG_WARNING, "UNKNOWN msg of type %d\n", hdr.type);
> -return 0; // return -1;
> +return -1;
>  }
>  if (hdr.size >= sizeof(msg)) {
>  syslog(LOG_WARNING,
> "msg size (%d) is too long (longer than %lu)\n",
> hdr.size, sizeof(msg));
> -return 0; // return -1;
> +return -1;
>  }
>  n = read(streamfd, , hdr.size);
>  if (n != hdr.size) {

Hi, why not, but why not skip this and use the exceptions straight
away?

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

[Spice-devel] [PATCH spice-streaming-agent 1/6] Be more restrictive about error handling

[Frediano Ziglio]

There's no much point in ignoring the error if these errors cause the
communication to be out of sync.
Ignoring them just change the state to indetermidate.

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

diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp
index 45a92b9..1f41a6f 100644
--- a/src/spice-streaming-agent.cpp
+++ b/src/spice-streaming-agent.cpp
@@ -94,17 +94,17 @@ static int read_command_from_device(void)
 if (hdr.protocol_version != STREAM_DEVICE_PROTOCOL) {
 syslog(LOG_WARNING, "BAD VERSION %d (expected is %d)\n", 
hdr.protocol_version,
STREAM_DEVICE_PROTOCOL);
-return 0; // return -1; -- fail over this ?
+return -1;
 }
 if (hdr.type != STREAM_TYPE_START_STOP) {
 syslog(LOG_WARNING, "UNKNOWN msg of type %d\n", hdr.type);
-return 0; // return -1;
+return -1;
 }
 if (hdr.size >= sizeof(msg)) {
 syslog(LOG_WARNING,
"msg size (%d) is too long (longer than %lu)\n",
hdr.size, sizeof(msg));
-return 0; // return -1;
+return -1;
 }
 n = read(streamfd, , hdr.size);
 if (n != hdr.size) {
-- 
2.14.3

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