Re: asio and kqueue (2nd trye) (was: RE: (boost::)asio and kqueue problem)

On Fri, Oct 14, 2016 at 09:21:52AM +0000, Hartmut.Brandt_at_dlr.de wrote:
> Hi all,
> 
> here is the 2nd try taking into account the comments I received. Since I'm not familiar with the locking in the sockets area I ask somebody with that knowledge to check it before I commit it.

I have only style notes, the factual code changes in the patch look good
to me.

Index: uipc_socket.c
===================================================================
--- uipc_socket.c	(revision 307091)
+++ uipc_socket.c	(working copy)
_at__at_ -159,15 +159,9 _at__at_
 static int	filt_soread(struct knote *kn, long hint);
 static void	filt_sowdetach(struct knote *kn);
 static int	filt_sowrite(struct knote *kn, long hint);
-static int	filt_solisten(struct knote *kn, long hint);
 static int inline hhook_run_socket(struct socket *so, void *hctx, int32_t h_id);
 fo_kqfilter_t	soo_kqfilter;
 
-static struct filterops solisten_filtops = {
-	.f_isfd = 1,
-	.f_detach = filt_sordetach,
-	.f_event = filt_solisten,
-};
 static struct filterops soread_filtops = {
 	.f_isfd = 1,
 	.f_detach = filt_sordetach,
_at__at_ -3075,10 +3069,7 _at__at_
 
 	switch (kn->kn_filter) {
 	case EVFILT_READ:
-		if (so->so_options & SO_ACCEPTCONN)
-			kn->kn_fop = &solisten_filtops;
-		else
-			kn->kn_fop = &soread_filtops;
+		kn->kn_fop = &soread_filtops;
 		sb = &so->so_rcv;
 		break;
 	case EVFILT_WRITE:
_at__at_ -3282,29 +3273,34 _at__at_
 static int
 filt_soread(struct knote *kn, long hint)
 {
-	struct socket *so;
+	struct socket *so = kn->kn_fp->f_data;
Style is against mixing declaration and initialization.  Please keep the
next removed line instead.
 
-	so = kn->kn_fp->f_data;
This one.

-	SOCKBUF_LOCK_ASSERT(&so->so_rcv);
+	if (so->so_options & SO_ACCEPTCONN) {
+		kn->kn_data = so->so_qlen;
+		return (!TAILQ_EMPTY(&so->so_comp));
 
-	kn->kn_data = sbavail(&so->so_rcv) - so->so_rcv.sb_ctl;
-	if (so->so_rcv.sb_state & SBS_CANTRCVMORE) {
-		kn->kn_flags |= EV_EOF;
-		kn->kn_fflags = so->so_error;
-		return (1);
-	} else if (so->so_error)	/* temporary udp error */
-		return (1);
+	} else {
You do not need else {} block, 'then' branch ends with return(). If you
remove else, you do not need additional indent for the old filt_soread()
function' body.

+		SOCKBUF_LOCK_ASSERT(&so->so_rcv);
 
-	if (kn->kn_sfflags & NOTE_LOWAT) {
-		if (kn->kn_data >= kn->kn_sdata)
-			return 1;
-	} else {
-		if (sbavail(&so->so_rcv) >= so->so_rcv.sb_lowat)
-			return 1;
+		kn->kn_data = sbavail(&so->so_rcv) - so->so_rcv.sb_ctl;
+		if (so->so_rcv.sb_state & SBS_CANTRCVMORE) {
+			kn->kn_flags |= EV_EOF;
+			kn->kn_fflags = so->so_error;
+			return (1);
+		} else if (so->so_error)	/* temporary udp error */
+			return (1);
+
+		if (kn->kn_sfflags & NOTE_LOWAT) {
+			if (kn->kn_data >= kn->kn_sdata)
+				return 1;
return (1);
since you change the line anyway.

+		} else {
+			if (sbavail(&so->so_rcv) >= so->so_rcv.sb_lowat)
+				return 1;
Same.

+		}
+
+		/* This hook returning non-zero indicates an event, not error */
+		return (hhook_run_socket(so, NULL, HHOOK_FILT_SOREAD));
 	}
-
-	/* This hook returning non-zero indicates an event, not error */
-	return (hhook_run_socket(so, NULL, HHOOK_FILT_SOREAD));
 }
 
 static void
_at__at_ -3346,16 +3342,6 _at__at_
 		return (kn->kn_data >= so->so_snd.sb_lowat);
 }
 
-/*ARGSUSED*/
-static int
-filt_solisten(struct knote *kn, long hint)
-{
-	struct socket *so = kn->kn_fp->f_data;
-
-	kn->kn_data = so->so_qlen;
-	return (!TAILQ_EMPTY(&so->so_comp));
-}
-
 int
 socheckuid(struct socket *so, uid_t uid)
 {