[Xcb] [PATCH 5/8] Add support for receiving fds in replies

[Uli Schlachter]

On 07.11.2013 04:45, Keith Packard wrote:
> Requests signal which replies will have fds, and the replies report
> how many fds they expect in byte 1.
> 
> Signed-off-by: Keith Packard <keithp at keithp.com>
[...]
> @@ -432,6 +463,11 @@ void *xcb_wait_for_reply(xcb_connection_t *c, unsigned int request, xcb_generic_
>      return ret;
>  }
>  
> +int *xcb_get_reply_fds(xcb_connection_t *c, void *reply, size_t reply_size)
> +{
> +    return (int *) (&((char *) reply)[reply_size]);
> +}
> +
>  static void insert_pending_discard(xcb_connection_t *c, pending_reply **prev_next, uint64_t seq)
>  {
>      pending_reply *pend;
> @@ -666,11 +702,68 @@ void _xcb_in_replies_done(xcb_connection_t *c)
>  
>  int _xcb_in_read(xcb_connection_t *c)
>  {
> -    int n = recv(c->fd, c->in.queue + c->in.queue_len, sizeof(c->in.queue) - c->in.queue_len, 0);
> -    if(n > 0)
> +    int n;
> +
> +#if HAVE_SENDMSG
> +    struct iovec    iov = {
> +        .iov_base = c->in.queue + c->in.queue_len,
> +        .iov_len = sizeof(c->in.queue) - c->in.queue_len,
> +    };
> +    struct msghdr msg = {
> +        .msg_name = NULL,
> +        .msg_namelen = 0,
> +        .msg_iov = &iov,
> +        .msg_iovlen = 1,
> +        .msg_control = &c->in.in_fd,
> +        .msg_controllen = sizeof (struct cmsghdr) + sizeof (int) * XCB_MAX_PASS_FD,
> +    };
> +    n = recvmsg(c->fd, &msg, 0);
>
> +
> +    /* Check for truncation errors. Only MSG_CTRUNC is
> +     * probably possible here, which would indicate that
> +     * the sender tried to transmit more than XCB_MAX_PASS_FD
> +     * file descriptors.
> +     */
> +    if (msg.msg_flags & (MSG_TRUNC|MSG_CTRUNC)) {
> +        _xcb_conn_shutdown(c, XCB_CONN_ERROR);
> +        return 0;

Could you introduce a new error value for this case, please? It would already be
quite hard to figure out that the connection was closed due to such an error and
without a clear error value, it will be quite impossible.

> +    }
> +#else
> +    n = recv(c->fd, c->in.queue + c->in.queue_len, sizeof(c->in.queue) - c->in.queue_len, 0);
> +#endif
> +    if(n > 0) {
> +#if HAVE_SENDMSG
> +        if (msg.msg_controllen > sizeof (struct cmsghdr))
> +        {
> +            if (c->in.in_fd.cmsghdr.cmsg_level == SOL_SOCKET &&
> +                c->in.in_fd.cmsghdr.cmsg_type == SCM_RIGHTS &&
> +                !((msg.msg_flags & MSG_TRUNC) ||
> +                  (msg.msg_flags & MSG_CTRUNC)))
> +            {
> +                c->in.in_fd.nfd = (msg.msg_controllen - sizeof (struct cmsghdr)) / sizeof (int);
> +            }
> +        }
> +#endif
>          c->in.queue_len += n;
> +    }
>      while(read_packet(c))
>          /* empty */;
> +#if HAVE_SENDMSG
> +    c->in.in_fd.nfd -= c->in.in_fd.ifd;
> +    c->in.in_fd.ifd = 0;
> +
> +    /* If we have any left-over file descriptors, then
> +     * the server sent some that we weren't expecting.
> +     * Close them and mark the connection as broken;
> +     */
> +    if (c->in.in_fd.nfd != 0) {
> +        int i;
> +        for (i = 0; i < c->in.in_fd.nfd; i++)
> +            close(c->in.in_fd.fd[i]);
> +        _xcb_conn_shutdown(c, XCB_CONN_ERROR);

I don't like this to be a generic error, too. Could this get the same error
value as the above _xcb_conn_shutdown()? Something like
XCB_CONN_CLOSED_FDPASSING_FAILED (no, I don't really like that name a lot, but I
don't have any better ideas right now).

> +        return 0;
> +    }
> +#endif
>  #ifndef _WIN32
>      if((n > 0) || (n < 0 && errno == EAGAIN))
>  #else
> diff --git a/src/xcbext.h b/src/xcbext.h
> index 44030c3..1eb1be7 100644
> --- a/src/xcbext.h
> +++ b/src/xcbext.h
> @@ -54,7 +54,8 @@ typedef struct {
>  enum xcb_send_request_flags_t {
>      XCB_REQUEST_CHECKED = 1 << 0,
>      XCB_REQUEST_RAW = 1 << 1,
> -    XCB_REQUEST_DISCARD_REPLY = 1 << 2
> +    XCB_REQUEST_DISCARD_REPLY = 1 << 2,
> +    XCB_REQUEST_REPLY_FDS = 1 << 3
>  };
>  
>  unsigned int xcb_send_request(xcb_connection_t *c, int flags, struct iovec *vector, const xcb_protocol_request_t *request);
> @@ -91,6 +92,7 @@ int xcb_writev(xcb_connection_t *c, struct iovec *vector, int count, uint64_t re
>  
>  void *xcb_wait_for_reply(xcb_connection_t *c, unsigned int request, xcb_generic_error_t **e);
>  int xcb_poll_for_reply(xcb_connection_t *c, unsigned int request, void **reply, xcb_generic_error_t **error);
> +int *xcb_get_reply_fds(xcb_connection_t *c, void *reply, size_t replylen);
>  
>  
>  /* xcb_util.c */
> diff --git a/src/xcbint.h b/src/xcbint.h
> index b2dfed3..bbc5398 100644
> --- a/src/xcbint.h
> +++ b/src/xcbint.h
> @@ -88,6 +88,7 @@ typedef struct _xcb_fd {
>      struct cmsghdr cmsghdr;
>      int fd[XCB_MAX_PASS_FD];
>      int nfd;
> +    int ifd;
>  } _xcb_fd;
>  #endif
>  
> @@ -146,6 +147,9 @@ typedef struct _xcb_in {
>  
>      struct pending_reply *pending_replies;
>      struct pending_reply **pending_replies_tail;
> +#if HAVE_SENDMSG
> +    _xcb_fd in_fd;
> +#endif
>  } _xcb_in;
>  
>  int _xcb_in_init(_xcb_in *in);
> 


-- 
"In the beginning the Universe was created. This has made a lot of
 people very angry and has been widely regarded as a bad move."