Patchwork [2/5] use qemu_pipe_non_block

login
register
mail settings
Submitter Alon Levy
Date June 4, 2013, 8:23 p.m.
Message ID <1370377419-31788-2-git-send-email-alevy@redhat.com>
Download mbox | patch
Permalink /patch/248853/
State New
Headers show

Comments

Alon Levy - June 4, 2013, 8:23 p.m.
This fixes six instances of unchecked fcntl return status, spotted by
Coverity.

Signed-off-by: Alon Levy <alevy@redhat.com>
---
 hw/display/qxl.c            | 10 +++-------
 hw/usb/ccid-card-emulated.c |  8 +++-----
 2 files changed, 6 insertions(+), 12 deletions(-)
Paolo Bonzini - June 4, 2013, 8:47 p.m.
Il 04/06/2013 22:23, Alon Levy ha scritto:
> This fixes six instances of unchecked fcntl return status, spotted by
> Coverity.

I think we're just assuming that they cannot fail...  I don't think we
need the previous patch and this one, unless they help porting stuff to
Windows.

Paolo
Peter Maydell - June 4, 2013, 8:50 p.m.
On 4 June 2013 21:23, Alon Levy <alevy@redhat.com> wrote:
> --- a/hw/display/qxl.c
> +++ b/hw/display/qxl.c
> @@ -1797,15 +1797,11 @@ static void qxl_send_events(PCIQXLDevice *d, uint32_t events)
>
>  static void init_pipe_signaling(PCIQXLDevice *d)
>  {
> -    if (pipe(d->pipe) < 0) {
> -        fprintf(stderr, "%s:%s: qxl pipe creation failed\n",
> -                __FILE__, __func__);
> +    if (qxl_pipe_non_block(d->pipe)) {

Surely this can't compile? -- this function doesn't exist.

thanks
-- PMM
Alon Levy - June 4, 2013, 8:56 p.m.
> On 4 June 2013 21:23, Alon Levy <alevy@redhat.com> wrote:
> > --- a/hw/display/qxl.c
> > +++ b/hw/display/qxl.c
> > @@ -1797,15 +1797,11 @@ static void qxl_send_events(PCIQXLDevice *d,
> > uint32_t events)
> >
> >  static void init_pipe_signaling(PCIQXLDevice *d)
> >  {
> > -    if (pipe(d->pipe) < 0) {
> > -        fprintf(stderr, "%s:%s: qxl pipe creation failed\n",
> > -                __FILE__, __func__);
> > +    if (qxl_pipe_non_block(d->pipe)) {
> 
> Surely this can't compile? -- this function doesn't exist.

I am abusing my right to post to this list, sorry. I didn't actually try to compile.

> 
> thanks
> -- PMM
> 
>
Alon Levy - June 4, 2013, 8:59 p.m.
> Il 04/06/2013 22:23, Alon Levy ha scritto:
> > This fixes six instances of unchecked fcntl return status, spotted by
> > Coverity.
> 
> I think we're just assuming that they cannot fail...  I don't think we
> need the previous patch and this one, unless they help porting stuff to
> Windows.

This was purely to satisfy coverity, but I thought we would want to check fcntl return status? also, shouldn't we be looping if EINTR?

> 
> Paolo
> 
>
Peter Maydell - June 4, 2013, 9:08 p.m.
On 4 June 2013 21:59, Alon Levy <alevy@redhat.com> wrote:
> shouldn't we be looping if EINTR?

Don't open that can of worms if you value your sanity ;-)

-- PMM

Patch

diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index 9e5b7ad..25c8c5a 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -1797,15 +1797,11 @@  static void qxl_send_events(PCIQXLDevice *d, uint32_t events)
 
 static void init_pipe_signaling(PCIQXLDevice *d)
 {
-    if (pipe(d->pipe) < 0) {
-        fprintf(stderr, "%s:%s: qxl pipe creation failed\n",
-                __FILE__, __func__);
+    if (qxl_pipe_non_block(d->pipe)) {
+        fprintf(stderr, "%s:%s: qxl pipe creation failed: %s\n",
+                __FILE__, __func__, stderror(errno));
         exit(1);
     }
-    fcntl(d->pipe[0], F_SETFL, O_NONBLOCK);
-    fcntl(d->pipe[1], F_SETFL, O_NONBLOCK);
-    fcntl(d->pipe[0], F_SETOWN, getpid());
-
     qemu_thread_get_self(&d->main);
     qemu_set_fd_handler(d->pipe[0], pipe_read, NULL, d);
 }
diff --git a/hw/usb/ccid-card-emulated.c b/hw/usb/ccid-card-emulated.c
index deb6d47..2e6942e 100644
--- a/hw/usb/ccid-card-emulated.c
+++ b/hw/usb/ccid-card-emulated.c
@@ -406,13 +406,11 @@  static void pipe_read(void *opaque)
 
 static int init_pipe_signaling(EmulatedState *card)
 {
-    if (pipe(card->pipe) < 0) {
-        DPRINTF(card, 2, "pipe creation failed\n");
+    if (qemu_pipe_non_block(card->pipe) < 0) {
+        DPRINTF(card, 2, "pipe creation failed: %s\n",
+                strerror(errno));
         return -1;
     }
-    fcntl(card->pipe[0], F_SETFL, O_NONBLOCK);
-    fcntl(card->pipe[1], F_SETFL, O_NONBLOCK);
-    fcntl(card->pipe[0], F_SETOWN, getpid());
     qemu_set_fd_handler(card->pipe[0], pipe_read, NULL, card);
     return 0;
 }