At Thu, 17 Jun 2010 18:18:18 +0100, Jamie Lokier wrote: > > 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.kazut...@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 >
I think It is another problem whether fgets() retries internally when a read system call is interrupted. We should handle EINTR if the system call can set EINTR. I think a read() doesn't return with EINTR if it doesn't block on Linux environment, but it may be not true on other operating systems. I send the fixed patch. I'm not sure this patch is really needed, but doesn't hurt anyway. = 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.kazut...@lab.ntt.co.jp> --- cmd.c | 14 +++++++++----- 1 files changed, 9 insertions(+), 5 deletions(-) diff --git a/cmd.c b/cmd.c index aee2a38..733bacd 100644 --- a/cmd.c +++ b/cmd.c @@ -293,14 +293,18 @@ fetchline(void) char * fetchline(void) { - char *p, *line = malloc(MAXREADLINESZ); + char *p, *line = malloc(MAXREADLINESZ), *ret; if (!line) return NULL; - if (!fgets(line, MAXREADLINESZ, stdin)) { - free(line); - return NULL; - } + do { + ret = fgets(line, MAXREADLINESZ, stdin); + } while (ret == NULL && errno == EINTR); + + if (ret == NULL) { + free(line); + return NULL; + } p = line + strlen(line); if (p != line && p[-1] == '\n') p[-1] = '\0'; -- 1.5.6.5