git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
* [PATCH v4 1/4] transport-helper: use xread instead of read
@ 2018-12-28 23:35 randall.s.becker
  2019-01-02 20:55 ` Junio C Hamano
  0 siblings, 1 reply; 4+ messages in thread
From: randall.s.becker @ 2018-12-28 23:35 UTC (permalink / raw)
  To: git; +Cc: Randall S. Becker

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) {
+ 	if (bytes < 0 && errno != EINTR) {
 		error_errno(_("read(%s) failed"), t->src_name);
-- 
2.17.0.10.gb132f7033


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v4 1/4] transport-helper: use xread instead of read
  2018-12-28 23:35 [PATCH v4 1/4] transport-helper: use xread instead of read randall.s.becker
@ 2019-01-02 20:55 ` Junio C Hamano
  2019-01-03  7:16   ` Jeff King
  0 siblings, 1 reply; 4+ messages in thread
From: Junio C Hamano @ 2019-01-02 20:55 UTC (permalink / raw)
  To: randall.s.becker; +Cc: git, Randall S. Becker

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.

Thanks.

> 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) {
> + 	if (bytes < 0 && errno != EINTR) {
>  		error_errno(_("read(%s) failed"), t->src_name);

Can't we also lose EINTR check, though?  When read() returns
negative, we check errno and if it is EINTR, continue the loop.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v4 1/4] transport-helper: use xread instead of read
  2019-01-02 20:55 ` Junio C Hamano
@ 2019-01-03  7:16   ` Jeff King
  2019-01-03 19:31     ` Junio C Hamano
  0 siblings, 1 reply; 4+ messages in thread
From: Jeff King @ 2019-01-03  7:16 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: randall.s.becker, git, Randall S. Becker

On Wed, Jan 02, 2019 at 12:55:51PM -0800, Junio C Hamano wrote:

> > 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) {
> > + 	if (bytes < 0 && errno != EINTR) {
> >  		error_errno(_("read(%s) failed"), t->src_name);
> 
> Can't we also lose EINTR check, though?  When read() returns
> negative, we check errno and if it is EINTR, continue the loop.

Yes.

I also wondered if this caller might actually be relying on the current
non-looping behavior, but it looks like I already traced through and
determined this was OK:

  https://public-inbox.org/git/20180111063110.GB31213@sigill.intra.peff.net/

(the cleanup is correct either way, but that is what makes the
conversion to xread() OK).

We may want to just take the xread() conversion and then do the patch
that I linked above on top, since it also cleans up the xwrite() spot,
too.

-Peff

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v4 1/4] transport-helper: use xread instead of read
  2019-01-03  7:16   ` Jeff King
@ 2019-01-03 19:31     ` Junio C Hamano
  0 siblings, 0 replies; 4+ messages in thread
From: Junio C Hamano @ 2019-01-03 19:31 UTC (permalink / raw)
  To: Jeff King; +Cc: randall.s.becker, git, Randall S. Becker

Jeff King <peff@peff.net> writes:

> On Wed, Jan 02, 2019 at 12:55:51PM -0800, Junio C Hamano wrote:
>
>> > 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) {
>> > + 	if (bytes < 0 && errno != EINTR) {
>> >  		error_errno(_("read(%s) failed"), t->src_name);
>> 
>> Can't we also lose EINTR check, though?  When read() returns
>> negative, we check errno and if it is EINTR, continue the loop.
>
> Yes.
>
> I also wondered if this caller might actually be relying on the current
> non-looping behavior, but it looks like I already traced through and
> determined this was OK:
>
>   https://public-inbox.org/git/20180111063110.GB31213@sigill.intra.peff.net/
>
> (the cleanup is correct either way, but that is what makes the
> conversion to xread() OK).
>
> We may want to just take the xread() conversion and then do the patch
> that I linked above on top, since it also cleans up the xwrite() spot,
> too.

OK.  That does sound cleaner.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-01-03 19:31 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-28 23:35 [PATCH v4 1/4] transport-helper: use xread instead of read randall.s.becker
2019-01-02 20:55 ` Junio C Hamano
2019-01-03  7:16   ` Jeff King
2019-01-03 19:31     ` Junio C Hamano

git@vger.kernel.org list mirror (unofficial, one of many)

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 git git/ https://public-inbox.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://7fh6tueqddpjyxjmgtdiueylzoqt6pt7hec3pukyptlmohoowvhde4yd.onion/inbox.comp.version-control.git
	nntp://ie5yzdi7fg72h7s4sdcztq5evakq23rdt33mfyfcddc5u3ndnw24ogqd.onion/inbox.comp.version-control.git
	nntp://4uok3hntl7oi7b4uf4rtfwefqeexfzil2w6kgk2jn5z2f764irre7byd.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git
 note: .onion URLs require Tor: https://www.torproject.org/

code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git