Re: kevent() not reporting closed slave pty (via master pty)

[Robert Elz <kre%munnari.OZ.AU@localhost>]

    Date:        Wed, 06 Feb 2019 20:29:32 +0100
    From:        =?UTF-8?Q?Micha=C5=82_G=C3=B3rny?= <mgorny%gentoo.org@localhost>
    Message-ID:  <1549481372.983.14.camel%gentoo.org@localhost>


  | +	} else if (!ISSET(tp->t_state, TS_ISOPEN) &&
  | +		   !ISSET(tp->t_state, TS_CARR_ON)) {
  | +	    kn->kn_flags |= EV_EOF;
  | +	    canread = 1;
  |  	}

The logic there looks incorrect, though I'd expect it to work
for your test case.   It might always work, and yet still not
really be correct (lead to readers of the code wondering why,
and perhaps copying this, elsewhere.)

TS_ISOPEN and TS_CARR_ON are (in this scenario) essentially
clones of each other.    Testing both should be superfluous.

They're both cleared when the slave pts is closed, and as
long as the master ptc remains open (which it does for your
test) they're both set again when the pts opens again.

The one time they differ, is when the pts opens with the ptc
closed (TS_ISOPEN is set, TS_CARR_ON is clear) but that should
be irrelevant/impossible when you're testing using an open ptc
fd (ie: we know the ptc is open.)

In ptcread() the read will return EOF (0) if !TS_CARR_ON
regardless of TS_ISOPEN (unless there is actually data to
be read).   The same is true in ptcpoll()
where
	revents |= POLLHUP;
if !TS_CARR_ON, again regardless of TS_ISOPEN.

It will take someone with far more kqueue clue than I have to
be sure (some pty and tty clues would help too) but I suspect
that all that might be needed is:

	if (!ISSET(tp->t_state, TS_CARR_ON)) {
		kn->kn_flags |= EV_EOF;
		canread = 1;
	}

(no "else" and no TS_ISOPEN test).    Testing TS_CARR_ON rather
that TS_ISOPEN merely for consistency with the other code.

Omitting the "else" is OK only if kqueue can handle the EOF
return, along with "this much data can be read" in the same
event.  Otherwise the "else" before the "if" should remain.

kre

ps: please indent the code in the body of the "if" to the same
level as the code in the "if" that precedes it (the "if (canread)"
not the one inside its block) (even if the "else" remains).

Deal with the line wrapping in the condition (if that os still required)
so it looks OK with the body code positioned first - there is more
latitude in how line wrapping is done than on indentation of blocks of code.