On Mon, Jan 25, 2016 at 11:22:13AM +0200, Boris Astardzhiev wrote:
> +ssize_t
> +recvmmsg(int s, struct mmsghdr *__restrict msgvec, size_t vlen, int flags,
> +    const struct timespec *__restrict timeout)
> +{
> +     size_t i, rcvd;
> +     ssize_t ret;
> +
> +     if (timeout != NULL) {
> +             fd_set fds;
> +             int res;
Please move all local definitions to the beginning of the function.

> +
> +             FD_ZERO(&fds);
> +             FD_SET(s, &fds);
> +             res = __sys_pselect(s + 1, &fds, NULL, NULL, timeout, NULL);
So why is this __sys_pselect() and not pselect() ?

Note that ppoll() is immune to the issue of s > FD_SETSIZE.

> +             if (res == -1 || res == 0)
> +                     return (res);
> +             if (!FD_ISSET(s, &fds))
> +                     return (-1);
> +     }
> +
> +     ret = __sys_recvmsg(s, &msgvec[0].msg_hdr, flags);
> +     if (ret == -1)
> +             return (ret);
> +
> +     /* Check initially for the presence of MSG_WAITFORONE.
> +      * Turn on MSG_DONTWAIT if set. */
The style-conformant multi-line comment is
        /*
         * Text.
         * More text.
         */
> +     if (flags & MSG_WAITFORONE) {
> +             flags |= MSG_DONTWAIT;
> +             /* The kernel doesn't need to know about this flag. */
> +             flags &= ~MSG_WAITFORONE;
You clear MSG_WAITFORONE flag in the calls to recvmsg(), but not at
the first recvmsg() incantation.  I think the flag should be just kept alone.

> +     }
> +
> +     rcvd = 1;
> +     for (i = rcvd; i < vlen; i++, rcvd++) {
> +             ret = __sys_recvmsg(s, &msgvec[i].msg_hdr, flags);
> +             if (ret == -1) {
> +                     /* We have received messages. Let caller know. */
> +                     return (rcvd);
> +             }
> +
> +             /* Save received bytes */
> +             msgvec[i].msg_len = ret;
> +     }
> +
> +     return (rcvd);
> +}

> diff --git a/sys/sys/socket.h b/sys/sys/socket.h
> index 18e2de1..c7a21cc 100644
> --- a/sys/sys/socket.h
> +++ b/sys/sys/socket.h
> @@ -431,6 +431,9 @@ struct msghdr {
>  #define      MSG_NBIO        0x4000          /* FIONBIO mode, used by fifofs 
> */
>  #define      MSG_COMPAT      0x8000          /* used in sendit() */
>  #define      MSG_CMSG_CLOEXEC 0x40000        /* make received fds 
> close-on-exec */
> +#ifndef _KERNEL
Why is the !KERNEL protection for the definition needed ?

> +#define MSG_WAITFORONE       0x80000         /* for recvmmsg() */
> +#endif /* !_KERNEL */
>  #endif
>  #ifdef _KERNEL
>  #define      MSG_SOCALLBCK   0x10000         /* for use by socket callbacks 
> - soreceive (TCP) */
> @@ -595,6 +598,18 @@ struct sf_hdtr {
>  #endif /* _KERNEL */
>  #endif /* __BSD_VISIBLE */
>  
> +#ifndef _KERNEL
Why is the !KERNEL protection for the definition needed ?

> +#ifdef __BSD_VISIBLE
> +/*
> + * Send/recvmmsg specific structure(s)
> + */
> +struct mmsghdr {
> +     struct msghdr   msg_hdr;                /* message header */
> +     ssize_t         msg_len;                /* message length */
> +};
> +#endif /* __BSD_VISIBLE */
> +#endif /* !_KERNEL */
> +
>  #ifndef      _KERNEL
>  
>  #include <sys/cdefs.h>
> @@ -615,11 +630,19 @@ int     listen(int, int);
>  ssize_t      recv(int, void *, size_t, int);
>  ssize_t      recvfrom(int, void *, size_t, int, struct sockaddr * 
> __restrict, socklen_t * __restrict);
>  ssize_t      recvmsg(int, struct msghdr *, int);
> +#if __BSD_VISIBLE
> +struct timespec;
> +ssize_t      recvmmsg(int, struct mmsghdr * __restrict, size_t, int,
> +    const struct timespec * __restrict);
> +#endif
>  ssize_t      send(int, const void *, size_t, int);
>  ssize_t      sendto(int, const void *,
>           size_t, int, const struct sockaddr *, socklen_t);
>  ssize_t      sendmsg(int, const struct msghdr *, int);
>  #if __BSD_VISIBLE
> +ssize_t      sendmmsg(int, struct mmsghdr * __restrict, size_t, int);
> +#endif
> +#if __BSD_VISIBLE
Again, there is no use in closing __BSD_VISIBLE block only to open
it again on the next line.

>  int  sendfile(int, int, off_t, size_t, struct sf_hdtr *, off_t *, int);
>  int  setfib(int);
>  #endif

_______________________________________________
freebsd-net@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"

Reply via email to