Re: PR/56569 CVS commit: src/crypto/external/bsd/openssh/dist

[Mike Small <smallm%sdf.org@localhost>]

"Christos Zoulas" <christos%netbsd.org@localhost> writes:

> The following reply was made to PR bin/56569; it has been noted by GNATS.
>
> From: "Christos Zoulas" <christos%netbsd.org@localhost>
> To: gnats-bugs%gnats.NetBSD.org@localhost
> Cc: 
> Subject: PR/56569 CVS commit: src/crypto/external/bsd/openssh/dist
> Date: Fri, 24 Dec 2021 13:16:11 -0500
>
>  Module Name:	src
>  Committed By:	christos
>  Date:		Fri Dec 24 18:16:11 UTC 2021
>  
>  Modified Files:
>  	src/crypto/external/bsd/openssh/dist: channels.c
>  
>  Log Message:
>  PR/56569: Mike Small: Remove unused code.
>  
>  
>  To generate a diff of this commit:
>  cvs rdiff -u -r1.32 -r1.33 src/crypto/external/bsd/openssh/dist/channels.c
>  
>  Please note that diffs are not public domain; they are subject to the
>  copyright notices on the relevant files.
>  

Sorry if I've missed something in the tip version or otherwise, but I'm
concerned that removing all of this will introduce a regression. It was
mostly dead code but not entirely in that it gave c->tcpwinsz an initial
value.

In recent HPN patches Chris Rapier changes that struct member to a local
variable which he populates before use in channel_check_window():
https://github.com/rapier1/openssh-portable/blob/master/channels.c

But that seems not the way in NetBSD code. I'm thinking the following
condition will now always be false since c->tcpwinsz will always be zero:

static int
channel_check_window(struct ssh *ssh, Channel *c)
{
...
		/* adjust max window size if we are in a dynamic environment */
		if (c->dynamic_window && (c->tcpwinsz > c->local_window_max)) {
			/* grow the window somewhat aggressively to maintain 
			 * pressure */
			addition = 1.5*(c->tcpwinsz - c->local_window_max);
			c->local_window_max += addition;
		}
...
		c->local_window += c->local_consumed + addition;
...
}

- Mike S.