Re: [Spice-devel] [PATCH spice-streaming-agent v2 1/2] Move reading stream messages into a function
On Thu, 2018-02-22 at 18:56 +0100, Christophe de Dinechin wrote: > > On 22 Feb 2018, at 18:41, Lukáš Hrázký wrote: > > > > Signed-off-by: Lukáš Hrázký > > --- > > src/spice-streaming-agent.cpp | 31 +-- > > 1 file changed, 21 insertions(+), 10 deletions(-) > > > > diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp > > index aeee5d3..20ac1f4 100644 > > --- a/src/spice-streaming-agent.cpp > > +++ b/src/spice-streaming-agent.cpp > > @@ -79,6 +79,24 @@ static int have_something_to_read(int timeout) > > return 0; > > } > > > > +static void read_all(void *msg, size_t len) > > +{ > > +size_t len_read = 0; > > + > > +while (len_read < len) { > > +ssize_t n = read(streamfd, (uint8_t *) msg + len_read, len - > > len_read); > > What about removing read_len, and decrementing len instead of incrementing > read_len? (also need to increment the pointer, obviously) That's a possibility, I just followed the structure of write_all()... > > + > > +if (n < 0) { > > +if (errno == EINTR) { > > +continue; > > +} > > +throw std::runtime_error("Reading message from device failed: > > " + > > + std::string(strerror(errno))); > > If this goes in before my own series, I will use that as an excuse to create > the IOError and ReadError classes, and avoid all this duplication of > strerror(errno) and string concatenation that bloats the code :-) No problem... > > +} > > +len_read += n; > > +} > > +} > > + > > static void handle_stream_start_stop(uint32_t len) > > { > > uint8_t msg[256]; > > @@ -87,11 +105,8 @@ static void handle_stream_start_stop(uint32_t len) > > throw std::runtime_error("msg size (" + std::to_string(len) + ") is > > too long " > > "(longer than " + > > std::to_string(sizeof(msg)) + ")"); > > } > > -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(len)); > > -} > > + > > +read_all(msg, len); > > streaming_requested = (msg[0] != 0); /* num_codecs */ > > syslog(LOG_INFO, "GOT START_STOP message -- request to %s streaming\n", > >streaming_requested ? "START" : "STOP"); > > @@ -108,12 +123,8 @@ static void handle_stream_capabilities(uint32_t len) > > if (len > sizeof(caps)) { > > throw std::runtime_error("capability message too long"); > > } > > -int n = read(streamfd, caps, len); > > -if (n != len) { > > -throw std::runtime_error("read command from device FAILED -- read > > " + std::to_string(n) + > > - " expected " + std::to_string(len)); > > -} > > > > +read_all(caps, len); > > // we currently do not support extensions so just reply so > > StreamDevHeader hdr = { > > STREAM_DEVICE_PROTOCOL, > > -- > > 2.16.1 > > > > ___ > > Spice-devel mailing list > > Spice-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/spice-devel > > ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-streaming-agent v2 1/2] Move reading stream messages into a function
> On 22 Feb 2018, at 18:41, Lukáš Hrázký wrote: > > Signed-off-by: Lukáš Hrázký > --- > src/spice-streaming-agent.cpp | 31 +-- > 1 file changed, 21 insertions(+), 10 deletions(-) > > diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp > index aeee5d3..20ac1f4 100644 > --- a/src/spice-streaming-agent.cpp > +++ b/src/spice-streaming-agent.cpp > @@ -79,6 +79,24 @@ static int have_something_to_read(int timeout) > return 0; > } > > +static void read_all(void *msg, size_t len) > +{ > +size_t len_read = 0; > + > +while (len_read < len) { > +ssize_t n = read(streamfd, (uint8_t *) msg + len_read, len - > len_read); What about removing read_len, and decrementing len instead of incrementing read_len? (also need to increment the pointer, obviously) > + > +if (n < 0) { > +if (errno == EINTR) { > +continue; > +} > +throw std::runtime_error("Reading message from device failed: " + > + std::string(strerror(errno))); If this goes in before my own series, I will use that as an excuse to create the IOError and ReadError classes, and avoid all this duplication of strerror(errno) and string concatenation that bloats the code :-) > +} > +len_read += n; > +} > +} > + > static void handle_stream_start_stop(uint32_t len) > { > uint8_t msg[256]; > @@ -87,11 +105,8 @@ static void handle_stream_start_stop(uint32_t len) > throw std::runtime_error("msg size (" + std::to_string(len) + ") is > too long " > "(longer than " + > std::to_string(sizeof(msg)) + ")"); > } > -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(len)); > -} > + > +read_all(msg, len); > streaming_requested = (msg[0] != 0); /* num_codecs */ > syslog(LOG_INFO, "GOT START_STOP message -- request to %s streaming\n", >streaming_requested ? "START" : "STOP"); > @@ -108,12 +123,8 @@ static void handle_stream_capabilities(uint32_t len) > if (len > sizeof(caps)) { > throw std::runtime_error("capability message too long"); > } > -int n = read(streamfd, caps, len); > -if (n != len) { > -throw std::runtime_error("read command from device FAILED -- read " > + std::to_string(n) + > - " expected " + std::to_string(len)); > -} > > +read_all(caps, len); > // we currently do not support extensions so just reply so > StreamDevHeader hdr = { > STREAM_DEVICE_PROTOCOL, > -- > 2.16.1 > > ___ > Spice-devel mailing list > Spice-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/spice-devel ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice-streaming-agent v2 1/2] Move reading stream messages into a function
Signed-off-by: Lukáš Hrázký --- src/spice-streaming-agent.cpp | 31 +-- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp index aeee5d3..20ac1f4 100644 --- a/src/spice-streaming-agent.cpp +++ b/src/spice-streaming-agent.cpp @@ -79,6 +79,24 @@ static int have_something_to_read(int timeout) return 0; } +static void read_all(void *msg, size_t len) +{ +size_t len_read = 0; + +while (len_read < len) { +ssize_t n = read(streamfd, (uint8_t *) msg + len_read, len - len_read); + +if (n < 0) { +if (errno == EINTR) { +continue; +} +throw std::runtime_error("Reading message from device failed: " + + std::string(strerror(errno))); +} +len_read += n; +} +} + static void handle_stream_start_stop(uint32_t len) { uint8_t msg[256]; @@ -87,11 +105,8 @@ static void handle_stream_start_stop(uint32_t len) throw std::runtime_error("msg size (" + std::to_string(len) + ") is too long " "(longer than " + std::to_string(sizeof(msg)) + ")"); } -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(len)); -} + +read_all(msg, len); streaming_requested = (msg[0] != 0); /* num_codecs */ syslog(LOG_INFO, "GOT START_STOP message -- request to %s streaming\n", streaming_requested ? "START" : "STOP"); @@ -108,12 +123,8 @@ static void handle_stream_capabilities(uint32_t len) if (len > sizeof(caps)) { throw std::runtime_error("capability message too long"); } -int n = read(streamfd, caps, len); -if (n != len) { -throw std::runtime_error("read command from device FAILED -- read " + std::to_string(n) + - " expected " + std::to_string(len)); -} +read_all(caps, len); // we currently do not support extensions so just reply so StreamDevHeader hdr = { STREAM_DEVICE_PROTOCOL, -- 2.16.1 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel