[Xcb] [PATCH libxcb] expose 64-bit sequence numbers for XLib

[Uli Schlachter]

Hi,

Am 21.03.2015 um 15:28 schrieb Christian Linhart:
> This is ABI and API compatible because it only adds new functions.
> 
> I will post the corresponding patch for Xlib to the xorg-devel list.

Could mention here:

The difference between xcb_discard_reply, xcb_wait_for_reply and
xcb_poll_for_reply and their uint64_t versions is just that a call do "widen()"
isn't made.

The difference between xcb_send_request and its 64 bit version is that only one
of them is actually implemented, but both are mentioned in a header. Perhaps
this should instead be fixed.

Also, this could say something about while these functions are necessary while
for the "preferred XCB API" unsigned int is enough.


As I see it, your patch to libX11 only uses the two *_for_reply() functions. For
xcb_send_request64(): I guess you just wanted this function for completeness,
but I'm not really sure about any use case. Since it's not implemented, I bet
that the implementation also isn't tested.
Same for xcb_discard_reply64(), but this function is quite useless: It requires
a 64 bit sequence number, but there is no (implemented) way to get one from
libxcb that can be used with this function. Its API docs explicitely forbid
socket handoff.

I'm unsure if we want those two functions or not. I'm fine with the 64 bit
versions of the _for_reply() functions.

Cheers,
Uli

> ---
>  src/xcb.h    | 15 +++++++++++++++
>  src/xcb_in.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
>  src/xcbext.h | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 109 insertions(+)
> 
> diff --git a/src/xcb.h b/src/xcb.h
> index 23fe74e..9a08fef 100644
> --- a/src/xcb.h
> +++ b/src/xcb.h
> @@ -378,6 +378,21 @@ xcb_generic_error_t *xcb_request_check(xcb_connection_t *c, xcb_void_cookie_t co
>   */
>  void xcb_discard_reply(xcb_connection_t *c, unsigned int sequence);
>  
> +/**
> + * @brief Discards the reply for a request, given by a 64bit sequence number
> + * @param c: The connection to the X server.
> + * @param sequence: The request sequence number from a cookie.
> + *
> + * Discards the reply for a request. Additionally, any error generated
> + * by the request is also discarded (unless it was an _unchecked request
> + * and the error has already arrived).
> + *
> + * This function will not block even if the reply is not yet available.
> + *
> + * Note that the sequence really does have to come from an xcb cookie;
> + * this function is not designed to operate on socket-handoff replies.
> + */
> +void xcb_discard_reply64(xcb_connection_t *c, uint64_t sequence);
>  
>  /* xcb_ext.c */
>  
> diff --git a/src/xcb_in.c b/src/xcb_in.c
> index ad870c1..623a0a8 100644
> --- a/src/xcb_in.c
> +++ b/src/xcb_in.c
> @@ -523,6 +523,20 @@ void *xcb_wait_for_reply(xcb_connection_t *c, unsigned int request, xcb_generic_
>      return ret;
>  }
>  
> +void *xcb_wait_for_reply64(xcb_connection_t *c, uint64_t request, xcb_generic_error_t **e)
> +{
> +    void *ret;
> +    if(e)
> +        *e = 0;
> +    if(c->has_error)
> +        return 0;
> +
> +    pthread_mutex_lock(&c->iolock);
> +    ret = wait_for_reply(c, request, e);
> +    pthread_mutex_unlock(&c->iolock);
> +    return ret;
> +}
> +
>  int *xcb_get_reply_fds(xcb_connection_t *c, void *reply, size_t reply_size)
>  {
>      return (int *) (&((char *) reply)[reply_size]);
> @@ -595,6 +609,20 @@ void xcb_discard_reply(xcb_connection_t *c, unsigned int sequence)
>      pthread_mutex_unlock(&c->iolock);
>  }
>  
> +void xcb_discard_reply64(xcb_connection_t *c, uint64_t sequence)
> +{
> +    if(c->has_error)
> +        return;
> +
> +    /* If an error occurred when issuing the request, fail immediately. */
> +    if(!sequence)
> +        return;
> +
> +    pthread_mutex_lock(&c->iolock);
> +    discard_reply(c, sequence);
> +    pthread_mutex_unlock(&c->iolock);
> +}
> +
>  int xcb_poll_for_reply(xcb_connection_t *c, unsigned int request, void **reply, xcb_generic_error_t **error)
>  {
>      int ret;
> @@ -612,6 +640,23 @@ int xcb_poll_for_reply(xcb_connection_t *c, unsigned int request, void **reply,
>      return ret;
>  }
>  
> +int xcb_poll_for_reply64(xcb_connection_t *c, uint64_t request, void **reply, xcb_generic_error_t **error)
> +{
> +    int ret;
> +    if(c->has_error)
> +    {
> +        *reply = 0;
> +        if(error)
> +            *error = 0;
> +        return 1; /* would not block */
> +    }
> +    assert(reply != 0);
> +    pthread_mutex_lock(&c->iolock);
> +    ret = poll_for_reply(c, request, reply, error);
> +    pthread_mutex_unlock(&c->iolock);
> +    return ret;
> +}
> +
>  xcb_generic_event_t *xcb_wait_for_event(xcb_connection_t *c)
>  {
>      xcb_generic_event_t *ret;
> diff --git a/src/xcbext.h b/src/xcbext.h
> index 7587513..25619aa 100644
> --- a/src/xcbext.h
> +++ b/src/xcbext.h
> @@ -83,6 +83,30 @@ 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, 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.
> + * @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 neccessary 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.
> + * 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_request64(xcb_connection_t *c, int flags, struct iovec *vector, const xcb_protocol_request_t *request);
> +
> +/**
>   * @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.
> @@ -162,6 +186,19 @@ 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);
>  
>  /**
> + * @brief Wait for the reply of a given request, with 64-bit sequence number
> + * @param c: The connection to the X server.
> + * @param request: Sequence number of the request as returned by xcb_send_request().
> + * @param e: Location to store errors in, or NULL. Ignored for unchecked requests.
> + *
> + * Returns the reply to the given request or returns null in the event of
> + * errors. Blocks until the reply or error for the request arrives, or an I/O
> + * error occurs.
> + */
> +
> +void *xcb_wait_for_reply64(xcb_connection_t *c, uint64_t request, xcb_generic_error_t **e);
> +
> +/**
>   * @brief Poll for the reply of a given request.
>   * @param c: The connection to the X server.
>   * @param request: Sequence number of the request as returned by xcb_send_request().
> @@ -174,6 +211,18 @@ void *xcb_wait_for_reply(xcb_connection_t *c, unsigned int request, xcb_generic_
>  int xcb_poll_for_reply(xcb_connection_t *c, unsigned int request, void **reply, xcb_generic_error_t **error);
>  
>  /**
> + * @brief Poll for the reply of a given request, with 64-bit sequence number.
> + * @param c: The connection to the X server.
> + * @param request: Sequence number of the request as returned by xcb_send_request().
> + * @param reply: Location to store the reply in, must not be NULL.
> + * @param e: Location to store errors in, or NULL. Ignored for unchecked requests.
> + * @return 1 when the reply to the request was returned, else 0.
> + *
> + * Checks if the reply to the given request already received. Does not block.
> + */
> +int xcb_poll_for_reply64(xcb_connection_t *c, uint64_t request, void **reply, xcb_generic_error_t **error);
> +
> +/**
>   * @brief Don't use this, only needed by the generated code.
>   * @param c: The connection to the X server.
>   * @param reply: A reply that was received from the server
> 


-- 
A learning experience is one of those things that say,
'You know that thing you just did? Don't do that.'
                     -- Douglas Adams