Re: [hackers] [sbase][PATCH] concat: read(2): handle EINTR

From: Richard Ipsum <richardipsum_AT_fastmail.co.uk>
Date: Sun, 23 Jul 2017 10:44:48 +0100

On Sun, Jul 23, 2017 at 11:18:59AM +0200, Mattias Andrée wrote:
> I don't think the #ifdef is necessary, EINTR if defined in POSIX
> and POSIX specifies that EINTR can be returned by read(3). Additionally,
> checking if EINTR is defined would be better.

That's interesting, I chose ifdef mostly because I'm only aware of
the Linux behaviour. My question then is if the ifdef is removed what
happens when a non-Linux platform executes this and returns EINTR?
Is that a fatal error for those platforms or can we recover?

>
> However, there is no need to check for EINTR unless the program
> catches signals. Is there any tools in sbase that both use concat
> and catch signals?

That seems to be the case for POSIX in general, but sadly not always for Linux.
My man page[1] has this to say,

       On Linux, even in the absence of signal handlers,
       certain blocking interfaces can fail with the error EINTR after the
       process is stopped by one of the stop signals and then resumed via
       SIGCONT. This behavior is not sanctioned by POSIX.1, and doesn't
       occur on other systems.

Luckily this seems to only apply if read(2) is reading from a file
descriptor returned by inotify(7), so the man page says. So I think
we agree that this could only ever be an issue if anything in sbase
is using concat while also catching signals.

Richard

[1]: https://linux.die.net/man/7/signal

>
>
> On Sun, 23 Jul 2017 10:13:50 +0100
> Richard Ipsum <richardipsum_AT_fastmail.co.uk> wrote:
>
> > On Linux we may receive EINTR if the read call is interrupted
> > before any data is read, if this is the case we can try to read
> > again.
> > ---
> > libutil/concat.c | 22 +++++++++++++++++-----
> > 1 file changed, 17 insertions(+), 5 deletions(-)
> >
> > diff --git a/libutil/concat.c b/libutil/concat.c
> > index 2e9aa52..5cbadd2 100644
> > --- a/libutil/concat.c
> > +++ b/libutil/concat.c
> > _AT_@ -1,4 +1,5 @@
> > /* See LICENSE file for copyright and license details. */
> > +#include <errno.h>
> > #include <unistd.h>
> >
> > #include "../util.h"
> > _AT_@ -9,15 +10,26 @@ concat(int f1, const char *s1, int f2, const char *s2)
> > char buf[BUFSIZ];
> > ssize_t n;
> >
> > - while ((n = read(f1, buf, sizeof(buf))) > 0) {
> > + for (;;) {
> > + n = read(f1, buf, sizeof(buf));
> > +
> > + if (n == 0)
> > + break;
> > +
> > + if (n < 0) {
> > +#ifdef __linux__
> > + if (errno == EINTR)
> > + continue;
> > +#endif
> > + weprintf("read %s:", s1);
> > + return -1;
> > + }
> > +
> > if (writeall(f2, buf, n) < 0) {
> > weprintf("write %s:", s2);
> > return -2;
> > }
> > }
> > - if (n < 0) {
> > - weprintf("read %s:", s1);
> > - return -1;
> > - }
> > +
> > return 0;
> > }
>



-- 
If I fail I try again, and again and again,
for as long as I try there's always a chance of
getting up. It's not the end until you've given up.
Received on Sun Jul 23 2017 - 11:44:48 CEST

This archive was generated by hypermail 2.3.0 : Sun Jul 23 2017 - 11:49:44 CEST