Hi Takashi,

On Fri, 27 Jun 2025, Takashi Yano wrote:

> On Fri, 27 Jun 2025 12:25:18 +0200 (CEST)
> Johannes Schindelin wrote:
> 
> > > Also, pipe_data_available() returns PDA_UNKNOWN rather than 1 when the
> > > pipe space estimation fails so that select() and raw_write() can perform
> > > appropriate fallback handling.
> > 
> > This looks unrelated? Would this not rather be in a separate patch, to
> > make it substantially easier to review for correctness?
> 
> I'll make a separate patch.

Thank you.

> > P.S.: One thing that strikes me as immediately concerning is this part:
> > 
> > > -   if (avail < 1)        /* error or pipe closed */
> > > +   if (avail == PDA_UNKNOWN && real_non_blocking_mode)
> > > +     avail = len1;
> > 
> > That means that the next loop iteration will call `NtWriteFile()` with
> > `len1` bytes (`len1` now being identical to `avail`), even if `len1` can
> > be substantially larger than `PIPE_BUF` (in my tests, it got stuck at
> > `len1 == 2097152` in some instances), which is highly likely to be
> > undesirable.
> 
> I don't think so. avail = len1 is performed only when real_non_blocking_mode.
> In real non blocking mode, we can call NtWriteFile() with larger data than
> actual pipe space without blocking.

That is a good explanation, and I see that you made it part of a commit
message in v2. Very good!

> When you observed `len1 == 2097152`, perhaps avail was 1, I guess.

Indeed.

Thank you,
Johannes

Reply via email to