[Xcb] [PATCH libxcb v3 3/5] Add xcb_send_request_with_fds() and *_with_fds64()

[Uli Schlachter]

Doing xcb_send_fd(), xcb_send_request() is racy. If two threads do this at the
same time, they could mix up their file descriptors. This commit makes it
possibly to fix this race by providing a single function which does everything
that is needed.

Signed-off-by: Uli Schlachter <psychon at znc.in>
---
 src/xcb_out.c | 22 +++++++++++++++--
 src/xcbext.h  | 77 +++++++++++++++++++++++++++++++++++++++++++++++++++++------
 2 files changed, 90 insertions(+), 9 deletions(-)

diff --git a/src/xcb_out.c b/src/xcb_out.c
index ec132fc..b5b6f34 100644
--- a/src/xcb_out.c
+++ b/src/xcb_out.c
@@ -214,15 +214,18 @@ static void send_fds(xcb_connection_t *c, int *fds, unsigned int num_fds)
     close_fds(fds, num_fds);
 }
 
-uint64_t xcb_send_request64(xcb_connection_t *c, int flags, struct iovec *vector, const xcb_protocol_request_t *req)
+uint64_t xcb_send_request_with_fds64(xcb_connection_t *c, int flags, struct iovec *vector,
+                const xcb_protocol_request_t *req, unsigned int num_fds, int *fds)
 {
     uint64_t request;
     uint32_t prefix[2];
     int veclen = req->count;
     enum workarounds workaround = WORKAROUND_NONE;
 
-    if(c->has_error)
+    if(c->has_error) {
+        close_fds(fds, num_fds);
         return 0;
+    }
 
     assert(c != 0);
     assert(vector != 0);
@@ -241,6 +244,7 @@ uint64_t xcb_send_request64(xcb_connection_t *c, int flags, struct iovec *vector
             const xcb_query_extension_reply_t *extension = xcb_get_extension_data(c, req->ext);
             if(!(extension && extension->present))
             {
+                close_fds(fds, num_fds);
                 _xcb_conn_shutdown(c, XCB_CONN_CLOSED_EXT_NOTSUPPORTED);
                 return 0;
             }
@@ -271,6 +275,7 @@ uint64_t xcb_send_request64(xcb_connection_t *c, int flags, struct iovec *vector
         }
         else if(longlen > xcb_get_maximum_request_length(c))
         {
+            close_fds(fds, num_fds);
             _xcb_conn_shutdown(c, XCB_CONN_CLOSED_REQ_LEN_EXCEED);
             return 0; /* server can't take this; maybe need BIGREQUESTS? */
         }
@@ -317,6 +322,7 @@ uint64_t xcb_send_request64(xcb_connection_t *c, int flags, struct iovec *vector
         prepare_socket_request(c);
     }
 
+    send_fds(c, fds, num_fds);
     send_request(c, req->isvoid, workaround, flags, vector, veclen);
     request = c->has_error ? 0 : c->out.request;
     pthread_mutex_unlock(&c->iolock);
@@ -324,6 +330,18 @@ uint64_t xcb_send_request64(xcb_connection_t *c, int flags, struct iovec *vector
 }
 
 /* request number are actually uint64_t internally but keep API compat with unsigned int */
+unsigned int xcb_send_request_with_fds(xcb_connection_t *c, int flags, struct iovec *vector,
+        const xcb_protocol_request_t *req, unsigned int num_fds, int *fds)
+{
+    return xcb_send_request_with_fds64(c, flags, vector, req, num_fds, fds);
+}
+
+uint64_t xcb_send_request64(xcb_connection_t *c, int flags, struct iovec *vector, const xcb_protocol_request_t *req)
+{
+    return xcb_send_request_with_fds64(c, flags, vector, req, 0, NULL);
+}
+
+/* request number are actually uint64_t internally but keep API compat with unsigned int */
 unsigned int xcb_send_request(xcb_connection_t *c, int flags, struct iovec *vector, const xcb_protocol_request_t *req)
 {
     return xcb_send_request64(c, flags, vector, req);
diff --git a/src/xcbext.h b/src/xcbext.h
index b2575f7..44d789e 100644
--- a/src/xcbext.h
+++ b/src/xcbext.h
@@ -68,13 +68,13 @@ enum xcb_send_request_flags_t {
  *
  * This function sends a new request to the X server. The data of the request is
  * given as an array of @c iovecs in the @p vector argument. The length of that
- * array and the neccessary management information are given in the @p request
+ * array and the necessary management information are given in the @p request
  * argument.
  *
  * When this function returns, the request might or might not be sent already.
  * Use xcb_flush() to make sure that it really was sent.
  *
- * Please note that this function is not the prefered way for sending requests.
+ * Please note that this function is not the preferred way for sending requests.
  * It's better to use the generated wrapper functions.
  *
  * Please note that xcb might use index -1 and -2 of the @p vector array internally,
@@ -83,6 +83,37 @@ enum xcb_send_request_flags_t {
 unsigned int xcb_send_request(xcb_connection_t *c, int flags, struct iovec *vector, const xcb_protocol_request_t *request);
 
 /**
+ * @brief Send a request to the server.
+ * @param c: The connection to the X server.
+ * @param flags: A combination of flags from the xcb_send_request_flags_t enumeration.
+ * @param vector: Data to send; must have two iovecs before start for internal use.
+ * @param request: Information about the request to be sent.
+ * @param num_fds: Number of additional file descriptors to send to the server
+ * @param fds: Additional file descriptors that should be send to the server.
+ * @return The request's sequence number on success, 0 otherwise.
+ *
+ * This function sends a new request to the X server. The data of the request is
+ * given as an array of @c iovecs in the @p vector argument. The length of that
+ * array and the necessary management information are given in the @p request
+ * argument.
+ *
+ * If @p num_fds is non-zero, @p fds points to an array of file descriptors that
+ * will be sent to the X server along with this request. After this function
+ * returns, all file descriptors sent are owned by xcb and will be closed
+ * eventually.
+ *
+ * When this function returns, the request might or might not be sent already.
+ * Use xcb_flush() to make sure that it really was sent.
+ *
+ * Please note that this function is not the preferred way for sending requests.
+ *
+ * Please note that xcb might use index -1 and -2 of the @p vector array internally,
+ * so they must be valid!
+ */
+unsigned int xcb_send_request_with_fds(xcb_connection_t *c, int flags, struct iovec *vector,
+                const xcb_protocol_request_t *request, unsigned int num_fds, int *fds);
+
+/**
  * @brief Send a request to the server, with 64-bit sequence number returned.
  * @param c: The connection to the X server.
  * @param flags: A combination of flags from the xcb_send_request_flags_t enumeration.
@@ -92,13 +123,13 @@ unsigned int xcb_send_request(xcb_connection_t *c, int flags, struct iovec *vect
  *
  * This function sends a new request to the X server. The data of the request is
  * given as an array of @c iovecs in the @p vector argument. The length of that
- * array and the neccessary management information are given in the @p request
+ * array and the necessary management information are given in the @p request
  * argument.
  *
  * When this function returns, the request might or might not be sent already.
  * Use xcb_flush() to make sure that it really was sent.
  *
- * Please note that this function is not the prefered way for sending requests.
+ * Please note that this function is not the preferred way for sending requests.
  * It's better to use the generated wrapper functions.
  *
  * Please note that xcb might use index -1 and -2 of the @p vector array internally,
@@ -107,6 +138,38 @@ unsigned int xcb_send_request(xcb_connection_t *c, int flags, struct iovec *vect
 uint64_t xcb_send_request64(xcb_connection_t *c, int flags, struct iovec *vector, const xcb_protocol_request_t *request);
 
 /**
+ * @brief Send a request to the server, with 64-bit sequence number returned.
+ * @param c: The connection to the X server.
+ * @param flags: A combination of flags from the xcb_send_request_flags_t enumeration.
+ * @param vector: Data to send; must have two iovecs before start for internal use.
+ * @param request: Information about the request to be sent.
+ * @param num_fds: Number of additional file descriptors to send to the server
+ * @param fds: Additional file descriptors that should be send to the server.
+ * @return The request's sequence number on success, 0 otherwise.
+ *
+ * This function sends a new request to the X server. The data of the request is
+ * given as an array of @c iovecs in the @p vector argument. The length of that
+ * array and the necessary management information are given in the @p request
+ * argument.
+ *
+ * If @p num_fds is non-zero, @p fds points to an array of file descriptors that
+ * will be sent to the X server along with this request. After this function
+ * returns, all file descriptors sent are owned by xcb and will be closed
+ * eventually.
+ *
+ * When this function returns, the request might or might not be sent already.
+ * Use xcb_flush() to make sure that it really was sent.
+ *
+ * Please note that this function is not the preferred way for sending requests.
+ * It's better to use the generated wrapper functions.
+ *
+ * Please note that xcb might use index -1 and -2 of the @p vector array internally,
+ * so they must be valid!
+ */
+uint64_t xcb_send_request_with_fds64(xcb_connection_t *c, int flags, struct iovec *vector,
+                const xcb_protocol_request_t *request, unsigned int num_fds, int *fds);
+
+/**
  * @brief Send a file descriptor to the server in the next call to xcb_send_request.
  * @param c: The connection to the X server.
  * @param fd: The file descriptor to send.
@@ -114,9 +177,9 @@ uint64_t xcb_send_request64(xcb_connection_t *c, int flags, struct iovec *vector
  * After this function returns, the file descriptor given is owned by xcb and
  * will be closed eventually.
  *
- * FIXME: How the heck is this supposed to work in a thread-safe way? There is a
- * race between two threads doing xcb_send_fd(); xcb_send_request(); at the same
- * time.
+ * @deprecated This function cannot be used in a thread-safe way. Two threads
+ * that run xcb_send_fd(); xcb_send_request(); could mix up their file
+ * descriptors. Instead, xcb_send_request_with_fds() should be used.
  */
 void xcb_send_fd(xcb_connection_t *c, int fd);
 
-- 
2.1.4