Looks cool.  Can you commit it?  Or should I?

* Maxim Konovalov <[EMAIL PROTECTED]> [050611 00:25] wrote:
> [ CC'ed rwatson ]
> 
> On Sat, 11 Jun 2005, 02:12+0400, Igor Sysoev wrote:
> 
> > Hi,
> >
> > man setsockopt(2) states that "passing in an optval of NULL will remove
> > the filter", however, setsockopt() always return EINVAL in this case,
> > because do_setopt_accept_filter() removes the filter if sopt == NULL, but
> > not if sopt->val == NULL.  The fix is easy:
> >
> > -        if (sopt == NULL) {
> > +        if (sopt == NULL || sopt->val == NULL) {
> 
> Yep, your observation is correct.  The second (mis)feature:
> getsockopt(SO_ACCEPTFILTER) always returns success on listen socket
> even we didn't install accept filter on the socket.
> 
> I extended the regression test for accept filter for these bugs.
> That makes the name of the test misleading though because now it tests
> detach case as well.
> 
> Index: sys/kern/uipc_accf.c
> ===================================================================
> RCS file: /home/ncvs/src/sys/kern/uipc_accf.c,v
> retrieving revision 1.18
> diff -u -p -r1.18 uipc_accf.c
> --- sys/kern/uipc_accf.c      12 Mar 2005 12:57:17 -0000      1.18
> +++ sys/kern/uipc_accf.c      11 Jun 2005 06:53:47 -0000
> @@ -176,8 +176,10 @@ do_getopt_accept_filter(struct socket *s
>               error = EINVAL;
>               goto out;
>       }
> -     if ((so->so_options & SO_ACCEPTFILTER) == 0)
> +     if ((so->so_options & SO_ACCEPTFILTER) == 0) {
> +             error = EINVAL;
>               goto out;
> +     }
>       strcpy(afap->af_name, so->so_accf->so_accept_filter->accf_name);
>       if (so->so_accf->so_accept_filter_str != NULL)
>               strcpy(afap->af_arg, so->so_accf->so_accept_filter_str);
> @@ -200,7 +202,7 @@ do_setopt_accept_filter(struct socket *s
>       /*
>        * Handle the simple delete case first.
>        */
> -     if (sopt == NULL) {
> +     if (sopt == NULL || sopt->sopt_val == NULL) {
>               SOCK_LOCK(so);
>               if ((so->so_options & SO_ACCEPTCONN) == 0) {
>                       SOCK_UNLOCK(so);
> Index: tools/regression/sockets/accf_data_attach/accf_data_attach.c
> ===================================================================
> RCS file: 
> /home/ncvs/src/tools/regression/sockets/accf_data_attach/accf_data_attach.c,v
> retrieving revision 1.3
> diff -u -p -r1.3 accf_data_attach.c
> --- tools/regression/sockets/accf_data_attach/accf_data_attach.c      11 Nov 
> 2004 19:47:53 -0000      1.3
> +++ tools/regression/sockets/accf_data_attach/accf_data_attach.c      11 Jun 
> 2005 07:06:41 -0000
> @@ -54,6 +54,8 @@
>   *   state.
>   * - That once an accept filter is attached, we can query to make sure it is
>   *   attached.
> + * - That once an accept filter is attached, we can remove it and query to
> + *   make sure it is removed.
>   */
>  int
>  main(int argc, char *argv[])
> @@ -145,38 +147,72 @@ main(int argc, char *argv[])
>       printf("ok 7 - listen\n");
> 
>       /*
> -      * Step 7: After listen().  This call to setsockopt() should succeed.
> +      * Step 7: Getsockopt() after listen().  Should fail with EINVAL,
> +      * since we have not installed accept filter yep.
> +      */
> +     len = sizeof(afa);
> +     ret = getsockopt(lso, SOL_SOCKET, SO_ACCEPTFILTER, &afa, &len);
> +     if (ret == 0)
> +             errx(-1, "not ok 8 - getsockopt() after listen() but before "
> +                 "setsockopt() succeeded");
> +     if (errno != EINVAL)
> +             errx(-1, "not ok 8 - getsockopt() after listen() but before "
> +                 "setsockopt() failed with %d (%s)", errno, strerror(errno));
> +     printf("ok 8 - getsockopt\n");
> +
> +     /*
> +      * Step 8: After listen().  This call to setsockopt() should succeed.
>        */
>       bzero(&afa, sizeof(afa));
>       strcpy(afa.af_name, ACCF_NAME);
>       ret = setsockopt(lso, SOL_SOCKET, SO_ACCEPTFILTER, &afa, sizeof(afa));
>       if (ret != 0)
> -             errx(-1, "not ok 8 - setsockopt() after listen() failed with %d 
> "
> +             errx(-1, "not ok 9 - setsockopt() after listen() failed with %d 
> "
>                   "(%s)", errno, strerror(errno));
>       if (len != sizeof(afa))
> -             errx(-1, "not ok 8 - setsockopt() after listen() returned wrong 
> "
> +             errx(-1, "not ok 9 - setsockopt() after listen() returned wrong 
> "
>                   "size (%d vs expected %d)", len, sizeof(afa));
> -     printf("ok 8 - setsockopt\n");
> +     printf("ok 9 - setsockopt\n");
> 
>       /*
> -      * Step 8: After setsockopt().  Should succeed and identify
> +      * Step 9: After setsockopt().  Should succeed and identify
>        * ACCF_NAME.
>        */
>       bzero(&afa, sizeof(afa));
>       len = sizeof(afa);
>       ret = getsockopt(lso, SOL_SOCKET, SO_ACCEPTFILTER, &afa, &len);
>       if (ret != 0)
> -             errx(-1, "not ok 9 - getsockopt() after listen() setsockopt() "
> +             errx(-1, "not ok 10 - getsockopt() after listen() setsockopt() "
>                   "failed with %d (%s)", errno, strerror(errno));
>       if (len != sizeof(afa))
> -             errx(-1, "not ok 9 - getsockopt() after setsockopet()  after "
> +             errx(-1, "not ok 10 - getsockopt() after setsockopet()  after "
>                   "listen() returned wrong size (got %d expected %d)", len,
>                   sizeof(afa));
>       if (strcmp(afa.af_name, ACCF_NAME) != 0)
> -             errx(-1, "not ok 9 - getsockopt() after setsockopt() after "
> +             errx(-1, "not ok 10 - getsockopt() after setsockopt() after "
>                   "listen() mismatch (got %s expected %s)", afa.af_name,
>                   ACCF_NAME);
> -     printf("ok 9 - getsockopt\n");
> +     printf("ok 10 - getsockopt\n");
> +
> +     /*
> +      * Step 10: Remove accept filter.  After removing the accept filter
> +      * getsockopt() should fail with EINVAL.
> +      */
> +     ret = setsockopt(lso, SOL_SOCKET, SO_ACCEPTFILTER, NULL, 0);
> +     if (ret != 0)
> +             errx(-1, "not ok 11 - setsockopt() after listen() "
> +                 "failed with %d (%s)", errno, strerror(errno));
> +     bzero(&afa, sizeof(afa));
> +     len = sizeof(afa);
> +     ret = getsockopt(lso, SOL_SOCKET, SO_ACCEPTFILTER, &afa, &len);
> +     if (ret == 0)
> +             errx(-1, "not ok 11 - getsockopt() after removing "
> +                 "the accept filter returns valid accept filter %s",
> +                 afa.af_name);
> +     if (errno != EINVAL)
> +             errx(-1, "not ok 11 - getsockopt() after removing the accept"
> +                 "filter failed with %d (%s)", errno, strerror(errno));
> +     printf("ok 11 - setsockopt\n");
> 
>       close(lso);
>       return (0);
> %%%
> 
> -- 
> Maxim Konovalov

-- 
- Alfred Perlstein
- email: [EMAIL PROTECTED] cell: 408-480-4684
_______________________________________________
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "[EMAIL PROTECTED]"

Reply via email to