[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/2] char: io_channel_send: don't lose written b
From: |
Anthony Liguori |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] char: io_channel_send: don't lose written bytes |
Date: |
Tue, 16 Jul 2013 13:57:46 -0500 |
User-agent: |
Notmuch/0.15.2+202~g0c4b8aa (http://notmuchmail.org) Emacs/23.3.1 (x86_64-pc-linux-gnu) |
Laszlo Ersek <address@hidden> writes:
> The g_io_channel_write_chars() documentation states,
>
> bytes_written: The number of bytes written. This can be nonzero even if
> the return value is not G_IO_STATUS_NORMAL. [...]
>
> io_channel_send() could lose such bytes before.
>
> Furthermore, the (status == G_IO_STATUS_EOF) condition used to evaluate to
> constant false whenever it was reached. When that condition actually held,
> it always led to -1 / EINVAL. This patch (almost) distinguishes
> G_IO_STATUS_EOF only when no bytes have been written, and then treats it
> as an error.
Just for my own benefit, I always assume G_IO_STATUS_EOF cannot happen
if bytes_written > 0. I see what you mean by the comment but do you
have any reason to believe this happens in practice?
Reviewed-by: Anthony Liguori <address@hidden>
Regards,
Anthony Liguori
>
> Signed-off-by: Laszlo Ersek <address@hidden>
> ---
> qemu-char.c | 41 +++++++++++++++++++----------------------
> 1 files changed, 19 insertions(+), 22 deletions(-)
>
> diff --git a/qemu-char.c b/qemu-char.c
> index 800d6a6..c86ce4b 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -720,35 +720,32 @@ static GIOChannel *io_channel_from_socket(int fd)
>
> static int io_channel_send(GIOChannel *fd, const void *buf, size_t len)
> {
> - GIOStatus status;
> - size_t offset;
> + size_t offset = 0;
> + GIOStatus status = G_IO_STATUS_NORMAL;
>
> - offset = 0;
> - while (offset < len) {
> - gsize bytes_written;
> + while (offset < len && status == G_IO_STATUS_NORMAL) {
> + gsize bytes_written = 0;
>
> status = g_io_channel_write_chars(fd, buf + offset, len - offset,
> &bytes_written, NULL);
> - if (status != G_IO_STATUS_NORMAL) {
> - if (status == G_IO_STATUS_AGAIN) {
> - /* If we've written any data, return a partial write. */
> - if (offset) {
> - break;
> - }
> - errno = EAGAIN;
> - } else {
> - errno = EINVAL;
> - }
> -
> - return -1;
> - } else if (status == G_IO_STATUS_EOF) {
> - break;
> - }
> -
> offset += bytes_written;
> }
>
> - return offset;
> + if (offset > 0) {
> + return offset;
> + }
> + switch (status) {
> + case G_IO_STATUS_NORMAL:
> + g_assert(len == 0);
> + return 0;
> + case G_IO_STATUS_AGAIN:
> + errno = EAGAIN;
> + return -1;
> + default:
> + break;
> + }
> + errno = EINVAL;
> + return -1;
> }
>
> #ifndef _WIN32
> --
> 1.7.1