On Tue, 2018-02-20 at 20:48 +0000, Frediano Ziglio wrote:
> In all paths errors from this function are treated like fatal
> error, there's no need to handle all manually potentially
> forgetting to handle errors.
> Also avoid to deal directly with logging moving the responsibility
> to other layers.
>
> Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> ---
> src/spice-streaming-agent.cpp | 35 ++++++++++++++---------------------
> 1 file changed, 14 insertions(+), 21 deletions(-)
>
> diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp
> index 1f41a6f..5613934 100644
> --- a/src/spice-streaming-agent.cpp
> +++ b/src/spice-streaming-agent.cpp
> @@ -77,7 +77,7 @@ static int have_something_to_read(int timeout)
> return 0;
> }
>
> -static int read_command_from_device(void)
> +static void read_command_from_device(void)
> {
> StreamDevHeader hdr;
> uint8_t msg[64];
> @@ -86,40 +86,32 @@ static int read_command_from_device(void)
> std::lock_guard<std::mutex> stream_guard(stream_mtx);
> n = read(streamfd, &hdr, sizeof(hdr));
> if (n != sizeof(hdr)) {
> - syslog(LOG_WARNING,
> - "read command from device FAILED -- read %d expected %lu\n",
> - n, sizeof(hdr));
> - return -1;
> + throw std::runtime_error("read command from device FAILED -- read " + std::to_string(n) +
> + " expected " + std::to_string(sizeof(hdr)));
> }
> if (hdr.protocol_version != STREAM_DEVICE_PROTOCOL) {
> - syslog(LOG_WARNING, "BAD VERSION %d (expected is %d)\n", hdr.protocol_version,
> - STREAM_DEVICE_PROTOCOL);
> - return -1;
> + 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) {
> - syslog(LOG_WARNING, "UNKNOWN msg of type %d\n", hdr.type);
> - return -1;
> + throw std::runtime_error("UNKNOWN msg of type " + std::to_string(hdr.type));
> }
> if (hdr.size >= sizeof(msg)) {
> - syslog(LOG_WARNING,
> - "msg size (%d) is too long (longer than %lu)\n",
> - hdr.size, sizeof(msg));
> - return -1;
> + throw std::runtime_error("msg size (" + std::to_string(hdr.size) + ") is too long "
> + "(longer than " + std::to_string(sizeof(msg)) + ")");
> }
> n = read(streamfd, &msg, hdr.size);
> if (n != hdr.size) {
> - syslog(LOG_WARNING,
> - "read command from device FAILED -- read %d expected %d\n",
> - n, hdr.size);
> - return -1;
> + throw std::runtime_error("read command from device FAILED -- read " + std::to_string(n) +
> + " expected " + std::to_string(hdr.size));
> }
> streaming_requested = (msg[0] != 0); /* num_codecs */
> syslog(LOG_INFO, "GOT START_STOP message -- request to %s streaming\n",
> streaming_requested ? "START" : "STOP");
> client_codecs.clear();
> - for (int i = 1; i <= msg[0]; ++i)
> + for (int i = 1; i <= msg[0]; ++i) {
> client_codecs.insert((SpiceVideoCodecType) msg[i]);
> - return 1;
> + }
Adding brackets, unrelated :) Just mentioning, I don't mind much.
> }
>
> static int read_command(bool blocking)
> @@ -133,7 +125,8 @@ static int read_command(bool blocking)
> sleep(1);
> continue;
> }
> - return read_command_from_device();
> + read_command_from_device();
> + break;
> }
>
> return 1;
I'm not entirely sure, if Christophe D still has anything to add to
this style of constructing exceptions. I'll give him the chance to say
it, for me:
Reviewed-by: Lukáš Hrázký <lhrazky@xxxxxxxxxx>
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel