Added more error states and removed global error_connection

Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=41443
Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=42304

I have added more xcb connection error states at xcb.h header.
Also I have removed global error_connection variable, and added
an interface that returns connection error state.

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.

Reviewed-by: Rami Ylimäki <rami.ylimaki@vincit.fi>
Reviewed-by: Uli Schlachter <psychon@znc.in>
Signed-off-by: Arvind Umrao <arvind.umrao@oracle.com>

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 errors because of socket, pipe 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))
 
 /* Opaque structures */
@@ -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);
 

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);

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;
         }
     }

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? */
         }
 

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;
     }
 

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);