| Message ID | 20181226230523.16572-2-randall.s.becker@rogers.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | HPE NonStop Port Commits | expand |
randall.s.becker@rogers.com writes: > From: "Randall S. Becker" <rsbecker@nexbridge.com> > > This fix was needed on HPE NonStop NSE and NSX where SSIZE_MAX is less than > BUFFERSIZE resulting in EINVAL. The call to read in transport-helper.c > was the only place outside of wrapper.c where it is used instead of xread. > > Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com> > --- > transport-helper.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/transport-helper.c b/transport-helper.c > index bf225c698f..a290695a12 100644 > --- a/transport-helper.c > +++ b/transport-helper.c > @@ -1225,7 +1225,7 @@ static int udt_do_read(struct unidirectional_transfer *t) > return 0; /* No space for more. */ > > transfer_debug("%s is readable", t->src_name); > - bytes = read(t->src, t->buf + t->bufuse, BUFFERSIZE - t->bufuse); > + bytes = xread(t->src, t->buf + t->bufuse, BUFFERSIZE - t->bufuse); > if (bytes < 0 && errno != EWOULDBLOCK && errno != EAGAIN && > errno != EINTR) { > error_errno(_("read(%s) failed"), t->src_name); As Peff pointed out in the earlier round of the same patch, replacing read() with xread() here will affect what errno's can be possible after the function returns. The checks affected by this change must also be updated, either in the same patch, or a follow-up patch in the same series. Otherwise we _will_ forget to clean them up.
On December 28, 2018 15:11, Junio C Hamano wrote: > randall.s.becker@rogers.com writes: > > > From: "Randall S. Becker" <rsbecker@nexbridge.com> > > > > This fix was needed on HPE NonStop NSE and NSX where SSIZE_MAX is less > > than BUFFERSIZE resulting in EINVAL. The call to read in > > transport-helper.c was the only place outside of wrapper.c where it is used > instead of xread. > > > > Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com> > > --- > > transport-helper.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/transport-helper.c b/transport-helper.c index > > bf225c698f..a290695a12 100644 > > --- a/transport-helper.c > > +++ b/transport-helper.c > > @@ -1225,7 +1225,7 @@ static int udt_do_read(struct > unidirectional_transfer *t) > > return 0; /* No space for more. */ > > > > transfer_debug("%s is readable", t->src_name); > > - bytes = read(t->src, t->buf + t->bufuse, BUFFERSIZE - t->bufuse); > > + bytes = xread(t->src, t->buf + t->bufuse, BUFFERSIZE - t->bufuse); > > if (bytes < 0 && errno != EWOULDBLOCK && errno != EAGAIN && > > errno != EINTR) { > > error_errno(_("read(%s) failed"), t->src_name); > > As Peff pointed out in the earlier round of the same patch, replacing read() > with xread() here will affect what errno's can be possible after the function > returns. The checks affected by this change must also be updated, either in > the same patch, or a follow-up patch in the same series. Otherwise we > _will_ forget to clean them up. If I read the xread code correctly, the change would be to leave EINTR and remove the other two conditions. Correct?
diff --git a/transport-helper.c b/transport-helper.c index bf225c698f..a290695a12 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -1225,7 +1225,7 @@ static int udt_do_read(struct unidirectional_transfer *t) return 0; /* No space for more. */ transfer_debug("%s is readable", t->src_name); - bytes = read(t->src, t->buf + t->bufuse, BUFFERSIZE - t->bufuse); + bytes = xread(t->src, t->buf + t->bufuse, BUFFERSIZE - t->bufuse); if (bytes < 0 && errno != EWOULDBLOCK && errno != EAGAIN && errno != EINTR) { error_errno(_("read(%s) failed"), t->src_name);