On Wed, 25 Jun 2025 14:55:35 +0200 (CEST) Johannes Schindelin wrote: > Hi Takashi, > > On Wed, 25 Jun 2025, Takashi Yano wrote: > > > Reviewed-by: > > Signed-off-by: Takashi Yano <[email protected]> > > This is way too terse. There is a difference between being succinct and > leaving things unsaid. > > Also, please make sure that v2 is a reply to v1 of the patch. I almost > commented on v1 by mistake. > > > --- > > winsup/cygwin/fhandler/pipe.cc | 10 ++-------- > > 1 file changed, 2 insertions(+), 8 deletions(-) > > > > diff --git a/winsup/cygwin/fhandler/pipe.cc b/winsup/cygwin/fhandler/pipe.cc > > index e35d523bb..c35411abf 100644 > > --- a/winsup/cygwin/fhandler/pipe.cc > > +++ b/winsup/cygwin/fhandler/pipe.cc > > @@ -443,7 +443,6 @@ ssize_t > > fhandler_pipe_fifo::raw_write (const void *ptr, size_t len) > > { > > size_t nbytes = 0; > > - ULONG chunk; > > Okay, removing this local variable is a good indicator that this diff > shows all the related logic, without having to resort to looking at the > entire `pipe.cc` file that is not reproduced in this email. > > > NTSTATUS status = STATUS_SUCCESS; > > IO_STATUS_BLOCK io; > > HANDLE evt; > > @@ -540,11 +539,6 @@ fhandler_pipe_fifo::raw_write (const void *ptr, size_t > > len) > > } > > } > > > > - if (len <= (size_t) avail) > > - chunk = len; > > - else > > - chunk = avail; > > - > > if (!(evt = CreateEvent (NULL, false, false, NULL))) > > { > > __seterrno (); > > @@ -561,8 +555,8 @@ fhandler_pipe_fifo::raw_write (const void *ptr, size_t > > len) > > ULONG len1; > > DWORD waitret = WAIT_OBJECT_0; > > > > - if (left > chunk && !is_nonblocking ()) > > - len1 = chunk; > > + if (left > (size_t) avail && !is_nonblocking ()) > > + len1 = (ULONG) avail; > > else > > len1 = (ULONG) left; > > So there is a subtle change here, which _should_ result in the same > behavior, but it is far from obvious.
Is that so? It seems abvious to me. Because... chunk has len (< avail) or avail. When left (= len - nbytes) > chunk, len > chunk. If len > chunk, chunk == avail. Isn't this obvious? Anyway, I'll add commit message and submit v3 patch after "SSH hang fix" is settled. -- Takashi Yano <[email protected]>
