[Xcb] [PATCH] Added more error states and removed global error_connection
Uli Schlachter
psychon at znc.in
Tue Nov 8 06:48:05 PST 2011
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
On 08.11.2011 12:22, Arvind Umrao wrote:
> As Rami Ylimäki suggested, I have removed errno=EPIPE adjustment.
>
> Also as per Uli Schlachter suggestion, I have declared and initialized error
> variables with file scope. I got reviewed-by tag from Rami. If no
> more suggestions, I will raise pull request.
Wow, thanks.
Patch looks good to me. I don't think my opinion matters much here, but since
you asked for it:
Reviewed-by: Uli Schlachter <psychon at znc.in>
(Plus some small comment below, but I don't care much about that one)
> TBD:
> I will segregate errors states in a separate header file and try to
> provide more precise error states, in future. Also I will give patch
> for libX11, in that patch xcb_connection_t::has_error will be passed
> to default io handler of libX11, This value can then be used for
> displaying error messages.
>
> Signed-off-by: Arvind Umrao <arvind.umrao at oracle.com>
> ---
> src/xcb.h | 24 +++++++++++++++++++++---
> src/xcb_conn.c | 45 ++++++++++++++++++++++++++++++++++++---------
> src/xcb_in.c | 14 +++++++-------
> src/xcb_out.c | 4 ++--
> src/xcb_util.c | 4 ++--
> src/xcbint.h | 7 ++++---
> 6 files changed, 72 insertions(+), 26 deletions(-)
>
> diff --git a/src/xcb.h b/src/xcb.h
> index 3ee7965..df74762 100644
> --- a/src/xcb.h
> +++ b/src/xcb.h
> @@ -69,6 +69,21 @@ extern "C" {
> /** X_TCP_PORT + display number = server port for TCP transport */
> #define X_TCP_PORT 6000
>
> +/** xcb connection shutdown because of connection errors for eg socket errors, pipe errors and other stream errors. */
> +#define XCB_CONN_ERROR 1
> +
> +/** xcb connection shutdown because of extension not sppported */
> +#define XCB_CONN_CLOSED_EXT_NOTSUPPORTED 2
> +
> +/** malloc(), calloc() and realloc() error upon failure, for eg ENOMEM */
> +#define XCB_CONN_CLOSED_MEM_INSUFFICIENT 3
> +
> +/** Connection closed, exceeding request length that server accepts. */
> +#define XCB_CONN_CLOSED_REQ_LEN_EXCEED 4
> +
> +/** Connection closed, error during parsing display string. */
> +#define XCB_CONN_CLOSED_PARSE_ERR 5
> +
> #define XCB_TYPE_PAD(T,I) (-(I) & (sizeof(T) > 4 ? 3 : sizeof(T) - 1))
Since this becomes part of the API, could someone who has a say double check if
these names are fine?
I'm just mentioning this since all these defines use a XCB_CONN_CLOSED_* prefix,
except XCB_CONN_ERROR. So perhaps XCB_CONN_CLOSED_SOCKER_ERROR?
> @@ -396,15 +411,18 @@ int xcb_get_file_descriptor(xcb_connection_t *c);
> /**
> * @brief Test whether the connection has shut down due to a fatal error.
> * @param c: The connection.
> - * @return 1 if the connection is in an error state; 0 otherwise.
> + * @return > 0 if the connection is in an error state; 0 otherwise.
> *
> * Some errors that occur in the context of an xcb_connection_t
> * are unrecoverable. When such an error occurs, the
> * connection is shut down and further operations on the
> * xcb_connection_t have no effect.
> *
> - * @todo Other functions should document the conditions in
> - * which they shut down the connection.
> + * @return XCB_CONN_ERROR, because of socket errors, pipe errors or other stream errors.
> + * @return XCB_CONN_CLOSED_EXT_NOTSUPPORTED, when extension not supported.
> + * @return XCB_CONN_CLOSED_MEM_INSUFFICIENT, when memory not available.
> + * @return XCB_CONN_CLOSED_REQ_LEN_EXCEED, exceeding request length that server accepts.
> + * @return XCB_CONN_CLOSED_PARSE_ERR, error during parsing display string.
> */
> int xcb_connection_has_error(xcb_connection_t *c);
>
> diff --git a/src/xcb_conn.c b/src/xcb_conn.c
> index 3ab5385..aa0fbde 100644
> --- a/src/xcb_conn.c
> +++ b/src/xcb_conn.c
> @@ -59,7 +59,9 @@ typedef struct {
> uint16_t length;
> } xcb_setup_generic_t;
>
> -const int error_connection = 1;
> +static const int xcb_con_error = XCB_CONN_ERROR;
> +static const int xcb_con_closed_mem_er = XCB_CONN_CLOSED_MEM_INSUFFICIENT;
> +static const int xcb_con_closed_parse_er = XCB_CONN_CLOSED_PARSE_ERR;
>
> static int set_fd_flags(const int fd)
> {
> @@ -209,7 +211,7 @@ static int write_vec(xcb_connection_t *c, struct iovec **vector, int *count)
>
> if(n <= 0)
> {
> - _xcb_conn_shutdown(c);
> + _xcb_conn_shutdown(c, XCB_CONN_ERROR);
> return 0;
> }
>
> @@ -263,7 +265,7 @@ xcb_connection_t *xcb_connect_to_fd(int fd, xcb_auth_info_t *auth_info)
> if(fd >= FD_SETSIZE) /* would overflow in FD_SET */
> {
> close(fd);
> - return (xcb_connection_t *) &error_connection;
> + return _xcb_conn_ret_error(XCB_CONN_ERROR);
> }
> #endif
> #endif /* !_WIN32*/
> @@ -271,7 +273,7 @@ xcb_connection_t *xcb_connect_to_fd(int fd, xcb_auth_info_t *auth_info)
> c = calloc(1, sizeof(xcb_connection_t));
> if(!c) {
> close(fd);
> - return (xcb_connection_t *) &error_connection;
> + return _xcb_conn_ret_error(XCB_CONN_CLOSED_MEM_INSUFFICIENT) ;
> }
>
> c->fd = fd;
> @@ -288,7 +290,7 @@ xcb_connection_t *xcb_connect_to_fd(int fd, xcb_auth_info_t *auth_info)
> ))
> {
> xcb_disconnect(c);
> - return (xcb_connection_t *) &error_connection;
> + return _xcb_conn_ret_error(XCB_CONN_ERROR);
> }
>
> return c;
> @@ -296,7 +298,7 @@ xcb_connection_t *xcb_connect_to_fd(int fd, xcb_auth_info_t *auth_info)
>
> void xcb_disconnect(xcb_connection_t *c)
> {
> - if(c == (xcb_connection_t *) &error_connection)
> + if(c->has_error)
> return;
>
> free(c->setup);
> @@ -317,9 +319,34 @@ void xcb_disconnect(xcb_connection_t *c)
>
> /* Private interface */
>
> -void _xcb_conn_shutdown(xcb_connection_t *c)
> +void _xcb_conn_shutdown(xcb_connection_t *c, int err)
> {
> - c->has_error = 1;
> + c->has_error = err;
> +}
> +
> +/* Return connection error state.
> + * To make thread-safe, I need a seperate static
> + * variable for every possible error.
> + */
> +xcb_connection_t *_xcb_conn_ret_error(int err)
> +{
> +
> + switch(err)
> + {
> + case XCB_CONN_CLOSED_MEM_INSUFFICIENT:
> + {
> + return (xcb_connection_t *) &xcb_con_closed_mem_er;
> + }
> + case XCB_CONN_CLOSED_PARSE_ERR:
> + {
> + return (xcb_connection_t *) &xcb_con_closed_parse_er;
> + }
> + case XCB_CONN_ERROR:
> + default:
> + {
> + return (xcb_connection_t *) &xcb_con_error;
> + }
> + }
> }
>
> int _xcb_conn_wait(xcb_connection_t *c, pthread_cond_t *cond, struct iovec **vector, int *count)
> @@ -380,7 +407,7 @@ int _xcb_conn_wait(xcb_connection_t *c, pthread_cond_t *cond, struct iovec **vec
> } while (ret == -1 && errno == EINTR);
> if(ret < 0)
> {
> - _xcb_conn_shutdown(c);
> + _xcb_conn_shutdown(c, XCB_CONN_ERROR);
> ret = 0;
> }
> pthread_mutex_lock(&c->iolock);
> diff --git a/src/xcb_in.c b/src/xcb_in.c
> index e075a40..969cfc0 100644
> --- a/src/xcb_in.c
> +++ b/src/xcb_in.c
> @@ -174,7 +174,7 @@ static int read_packet(xcb_connection_t *c)
> (genrep.response_type == XCB_REPLY ? 0 : sizeof(uint32_t)));
> if(!buf)
> {
> - _xcb_conn_shutdown(c);
> + _xcb_conn_shutdown(c, XCB_CONN_CLOSED_MEM_INSUFFICIENT);
> return 0;
> }
>
> @@ -210,7 +210,7 @@ static int read_packet(xcb_connection_t *c)
> struct reply_list *cur = malloc(sizeof(struct reply_list));
> if(!cur)
> {
> - _xcb_conn_shutdown(c);
> + _xcb_conn_shutdown(c, XCB_CONN_CLOSED_MEM_INSUFFICIENT);
> free(buf);
> return 0;
> }
> @@ -227,7 +227,7 @@ static int read_packet(xcb_connection_t *c)
> event = malloc(sizeof(struct event_list));
> if(!event)
> {
> - _xcb_conn_shutdown(c);
> + _xcb_conn_shutdown(c, XCB_CONN_CLOSED_MEM_INSUFFICIENT);
> free(buf);
> return 0;
> }
> @@ -433,7 +433,7 @@ static void insert_pending_discard(xcb_connection_t *c, pending_reply **prev_nex
> pend = malloc(sizeof(*pend));
> if(!pend)
> {
> - _xcb_conn_shutdown(c);
> + _xcb_conn_shutdown(c, XCB_CONN_CLOSED_MEM_INSUFFICIENT);
> return;
> }
>
> @@ -633,7 +633,7 @@ int _xcb_in_expect_reply(xcb_connection_t *c, uint64_t request, enum workarounds
> assert(workaround != WORKAROUND_NONE || flags != 0);
> if(!pend)
> {
> - _xcb_conn_shutdown(c);
> + _xcb_conn_shutdown(c, XCB_CONN_CLOSED_MEM_INSUFFICIENT);
> return 0;
> }
> pend->first_request = pend->last_request = request;
> @@ -672,7 +672,7 @@ int _xcb_in_read(xcb_connection_t *c)
> if((n > 0) || (n < 0 && WSAGetLastError() == WSAEWOULDBLOCK))
> #endif /* !_WIN32 */
> return 1;
> - _xcb_conn_shutdown(c);
> + _xcb_conn_shutdown(c, XCB_CONN_ERROR);
> return 0;
> }
>
> @@ -691,7 +691,7 @@ int _xcb_in_read_block(xcb_connection_t *c, void *buf, int len)
> int ret = read_block(c->fd, (char *) buf + done, len - done);
> if(ret <= 0)
> {
> - _xcb_conn_shutdown(c);
> + _xcb_conn_shutdown(c, XCB_CONN_ERROR);
> return ret;
> }
> }
> diff --git a/src/xcb_out.c b/src/xcb_out.c
> index 5eb1e42..2d3d005 100644
> --- a/src/xcb_out.c
> +++ b/src/xcb_out.c
> @@ -173,7 +173,7 @@ unsigned int xcb_send_request(xcb_connection_t *c, int flags, struct iovec *vect
> const xcb_query_extension_reply_t *extension = xcb_get_extension_data(c, req->ext);
> if(!(extension && extension->present))
> {
> - _xcb_conn_shutdown(c);
> + _xcb_conn_shutdown(c, XCB_CONN_CLOSED_EXT_NOTSUPPORTED);
> return 0;
> }
> ((uint8_t *) vector[0].iov_base)[0] = extension->major_opcode;
> @@ -203,7 +203,7 @@ unsigned int xcb_send_request(xcb_connection_t *c, int flags, struct iovec *vect
> }
> else if(longlen > xcb_get_maximum_request_length(c))
> {
> - _xcb_conn_shutdown(c);
> + _xcb_conn_shutdown(c, XCB_CONN_CLOSED_REQ_LEN_EXCEED);
> return 0; /* server can't take this; maybe need BIGREQUESTS? */
> }
>
> diff --git a/src/xcb_util.c b/src/xcb_util.c
> index fcb11f0..a55df16 100644
> --- a/src/xcb_util.c
> +++ b/src/xcb_util.c
> @@ -424,13 +424,13 @@ xcb_connection_t *xcb_connect_to_display_with_auth_info(const char *displayname,
> int parsed = _xcb_parse_display(displayname, &host, &protocol, &display, screenp);
>
> if(!parsed) {
> - c = (xcb_connection_t *) &error_connection;
> + c = _xcb_conn_ret_error(XCB_CONN_CLOSED_PARSE_ERR);
> goto out;
> } else
> fd = _xcb_open(host, protocol, display);
>
> if(fd == -1) {
> - c = (xcb_connection_t *) &error_connection;
> + c = _xcb_conn_ret_error(XCB_CONN_ERROR);
> goto out;
> }
>
> diff --git a/src/xcbint.h b/src/xcbint.h
> index 096576c..f9e5a52 100644
> --- a/src/xcbint.h
> +++ b/src/xcbint.h
> @@ -174,8 +174,6 @@ void _xcb_ext_destroy(xcb_connection_t *c);
>
> /* xcb_conn.c */
>
> -extern const int error_connection;
> -
> struct xcb_connection_t {
> int has_error;
>
> @@ -193,7 +191,10 @@ struct xcb_connection_t {
> _xcb_xid xid;
> };
>
> -void _xcb_conn_shutdown(xcb_connection_t *c);
> +void _xcb_conn_shutdown(xcb_connection_t *c, int err);
> +
> +xcb_connection_t *_xcb_conn_ret_error(int err);
> +
> int _xcb_conn_wait(xcb_connection_t *c, pthread_cond_t *cond, struct iovec **vector, int *count);
>
>
- --
"Do you know that books smell like nutmeg or some spice from a foreign land?"
-- Faber in Fahrenheit 451
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
iQEcBAEBCAAGBQJOuUEjAAoJECLkKOvLj8sG9VMIAMOOL5Qp7bz/ldum8b1AJK+4
K79ceLSU8kLxGcfpCwzMmCEcV4D+tBhDEMap3H+iErPmwaFqPdUDEXjkpnqfmLeX
m18RAz2jc+gAPOEQMpWm5rLUraFCJAJhc2vsHv9GioOwLED1SCSs9HCec7QYIMbg
TOnsC7if2MV7R+u+J9G1KjmvUTst3LR2kBxk/ifatjpOtB/jj+s2+317phYTqfVA
UTwLUQBAVBVr4U1c9YRWT9KDoiURScgHhmaf8GJRohwI9vcmzUlYXZM9X3h1T7Jg
Pwu5rC84x+l4LsJiFRBgrGdos9H1NeR6xgE26FImy62OTxqnSGwjkfbh4x6OriI=
=MBUv
-----END PGP SIGNATURE-----
More information about the Xcb
mailing list