On Sat, Dec 17, 2022 at 11:42:14AM -0700, Theo de Raadt wrote: > Vitaliy Makkoveev <[email protected]> wrote: > > > The reworked diff for `so_snd' SS_CANTSENDMORE state bit. As proposed by > > deraadt@, SS_CANTSENDMORE definition left as is, but used with newly > > introduced socket's buffer state `sb_state'. `sb_state' ored with > > original `so_state' when socket's data exported to the userland, so the > > ABI left as it was. > > > > The `so_rcv' related SS_CANTRCVMORE and SS_RCVATMARK bits left to the > > following diff. > > That looks better. I didn't run it yet. > > I think there should be a comments which says where each of the SS_* bits > are put and found. > > Some go into so_state, some are in sb_state. > > In the future, someone carelessly put a SS_* bit into the wrong variable, > and thus not satisfy a read condition on the other side. > > u_int so_state; /* internal state flags SS_*, below */ > > See this becomes somewhat innaccurate, someone might put one of the SS_ > bits you moved into here based upon the comment. Maybe it should say > "some of the SS_* flags */, and use the same comment for the new sb_state. > > It should be something subtle and small, to avoid line-wrapping the > #define SS_* lines. > > Maybe like this, killing a tab also, one line is more than 80... perhaps > that's just how it has to be? > > * Socket state bits. > * B - in sb_state, S - in so_state > */ > #define SS_NOFDREF 0x001 /* [S] no file table ref any more */ > #define SS_ISCONNECTED 0x002 /* [S] socket connected to a peer */ > #define SS_ISCONNECTING 0x004 /* [S] in process of connecting to peer > */ > #define SS_ISDISCONNECTING 0x008 /* [S] in process of disconnecting */ > #define SS_CANTSENDMORE 0x010 /* [B] can't send more data to peer */ > #define SS_CANTRCVMORE 0x020 /* [S] can't receive more data from > peer */ > #define SS_RCVATMARK 0x040 /* [S] at mark on input */ > #define SS_ISDISCONNECTED 0x800 /* [S] socket disconnected from peer */ > > and same for the further bits... >
What about to add "NOTE" before "Socket state bits" definition? /* * Socket state bits. + * + * NOTE: The following states should be used with corresponding socket's + * buffer `sb_state' only: + * + * SS_CANTSENDMORE with `so_snd' */ + #define SS_NOFDREF 0x001 /* no file table ref any more */ #define SS_ISCONNECTED 0x002 /* socket connected to a peer */ #define SS_ISCONNECTING 0x004 /* in process of connecting to peer */
