[Qemu-devel] [PATCH for-2.5] qemu-char: retry g_poll on EINTR

[Qemu-devel] [PATCH for-2.5] qemu-char: retry g_poll on EINTR

[Paolo Bonzini]

This is a case where pty_chr_update_read_handler_locked's lack
of error checking can produce incorrect values.  We are not using
SIGUSR1 anymore, so this is quite theoretical, but easy to fix.

Reported-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-char.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/qemu-char.c b/qemu-char.c
index 5448b0f..2969c44 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1241,11 +1241,16 @@ static void pty_chr_update_read_handler_locked(CharDriverState *chr)
 {
     PtyCharDriver *s = chr->opaque;
     GPollFD pfd;
+    int rc;
 
     pfd.fd = g_io_channel_unix_get_fd(s->fd);
     pfd.events = G_IO_OUT;
     pfd.revents = 0;
-    g_poll(&pfd, 1, 0);
+    do {
+        rc = g_poll(&pfd, 1, 0);
+    } while (rc == -1 && errno == EINTR);
+    assert(rc >= 0);
+
     if (pfd.revents & G_IO_HUP) {
         pty_chr_state(chr, 0);
     } else {
-- 
2.5.0

Re: [Qemu-devel] [PATCH for-2.5] qemu-char: retry g_poll on EINTR

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> This is a case where pty_chr_update_read_handler_locked's lack
> of error checking can produce incorrect values.  We are not using
> SIGUSR1 anymore, so this is quite theoretical, but easy to fix.
>
> Reported-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  qemu-char.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/qemu-char.c b/qemu-char.c
> index 5448b0f..2969c44 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -1241,11 +1241,16 @@ static void pty_chr_update_read_handler_locked(CharDriverState *chr)
>  {
>      PtyCharDriver *s = chr->opaque;
>      GPollFD pfd;
> +    int rc;
>  
>      pfd.fd = g_io_channel_unix_get_fd(s->fd);
>      pfd.events = G_IO_OUT;
>      pfd.revents = 0;
> -    g_poll(&pfd, 1, 0);
> +    do {
> +        rc = g_poll(&pfd, 1, 0);
> +    } while (rc == -1 && errno == EINTR);

Could use TFR(), but that's a matter of taste.  Both ways aleady exist
in this file.

> +    assert(rc >= 0);

Documented errors other than EINTR:

       EFAULT The array given as argument was not contained in the
              calling program's address space.

       EINVAL The nfds value exceeds the RLIMIT_NOFILE value.

       ENOMEM There was no space to allocate file descriptor tables.

Aborting feels okay to me.

> +
>      if (pfd.revents & G_IO_HUP) {
>          pty_chr_state(chr, 0);
>      } else {

Reviewed-by: Markus Armbruster <armbru@redhat.com>