All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jamie Lokier <jamie@shareable.org>
To: Kevin Wolf <kwolf@redhat.com>
Cc: qemu-devel@nongnu.org, MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
Subject: Re: [Qemu-devel] Re: [PATCH 1/2] qemu-io: retry fgets() when errno is	EINTRg
Date: Thu, 17 Jun 2010 18:18:18 +0100	[thread overview]
Message-ID: <20100617171818.GB4979@shareable.org> (raw)
In-Reply-To: <4C19DC88.4080208@redhat.com>

Kevin Wolf wrote:
> Am 16.06.2010 18:52, schrieb MORITA Kazutaka:
> > At Wed, 16 Jun 2010 13:04:47 +0200,
> > Kevin Wolf wrote:
> >>
> >> Am 15.06.2010 19:53, schrieb MORITA Kazutaka:
> >>> posix-aio-compat sends a signal in aio operations, so we should
> >>> consider that fgets() could be interrupted here.
> >>>
> >>> Signed-off-by: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
> >>> ---
> >>>  cmd.c |    3 +++
> >>>  1 files changed, 3 insertions(+), 0 deletions(-)
> >>>
> >>> diff --git a/cmd.c b/cmd.c
> >>> index 2336334..460df92 100644
> >>> --- a/cmd.c
> >>> +++ b/cmd.c
> >>> @@ -272,7 +272,10 @@ fetchline(void)
> >>>  		return NULL;
> >>>  	printf("%s", get_prompt());
> >>>  	fflush(stdout);
> >>> +again:
> >>>  	if (!fgets(line, MAXREADLINESZ, stdin)) {
> >>> +		if (errno == EINTR)
> >>> +			goto again;
> >>>  		free(line);
> >>>  		return NULL;
> >>>  	}
> >>
> >> This looks like a loop replaced by goto (and braces are missing). What
> >> about this instead?
> >>
> >> do {
> >>     ret = fgets(...)
> >> } while (ret == NULL && errno == EINTR)
> >>
> >> if (ret == NULL) {
> >>    fail
> >> }
> >>
> > 
> > I agree.
> > 
> > However, it seems that my second patch have already solved the
> > problem.  We register this readline routines as an aio handler now, so
> > fgets() does not block and cannot return with EINTR.
> > 
> > This patch looks no longer needed, sorry.
> 
> Good point. Thanks for having a look.

Anyway, are you sure stdio functions can be interrupted with EINTR?
Linus reminds us that some stdio functions have to retry internally
anyway:

http://comments.gmane.org/gmane.comp.version-control.git/18285

-- Jamie

  reply	other threads:[~2010-06-17 17:18 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-06-15 17:53 [Qemu-devel] [PATCH 0/2] qemu-io: fix aio_read/write problems MORITA Kazutaka
2010-06-15 17:53 ` [Qemu-devel] [PATCH 1/2] qemu-io: retry fgets() when errno is EINTR MORITA Kazutaka
2010-06-16 11:04   ` [Qemu-devel] " Kevin Wolf
2010-06-16 16:52     ` MORITA Kazutaka
2010-06-17  8:27       ` Kevin Wolf
2010-06-17 17:18         ` Jamie Lokier [this message]
2010-06-18  4:11           ` [Qemu-devel] Re: [PATCH 1/2] qemu-io: retry fgets() when errno is EINTRg MORITA Kazutaka
2010-06-15 17:53 ` [Qemu-devel] [PATCH 2/2] qemu-io: check registered fds in command_loop() MORITA Kazutaka
2010-06-16 11:24   ` [Qemu-devel] " Kevin Wolf
2010-06-18 16:21   ` Kevin Wolf
2010-06-20 19:03     ` [Qemu-devel] [PATCH v2] " MORITA Kazutaka
2010-06-21 10:39       ` [Qemu-devel] " Kevin Wolf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20100617171818.GB4979@shareable.org \
    --to=jamie@shareable.org \
    --cc=kwolf@redhat.com \
    --cc=morita.kazutaka@lab.ntt.co.jp \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.