On Fri, Oct 14, 2016 at 09:21:52AM +0000, hartmut.bra...@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)
@@ -159,15 +159,9 @@
 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,
@@ -3075,10 +3069,7 @@
 
        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:
@@ -3282,29 +3273,34 @@
 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
@@ -3346,16 +3342,6 @@
                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)
 {
_______________________________________________
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Reply via email to