Merge Damien's extentions of the client library into master-stable

Conflicts:
	test/box/protocol.c

Implement review fixes along the way (coding style, structure
the error handling in the client library differently).
Add test/box/protocol to .gitignore.
Produce more informative error messages in protocol.c/protocol.test.

.gitignore

@@ -13,6 +13,7 @@ lcov
 test/var
 test/lib/*.pyc
 test/lib/*/*.pyc
+test/box/protocol
 Makefile
 CMakeFiles
 CMakeCache.txt

connector/c/client.c

@@ -32,54 +32,106 @@
 #include <netinet/in.h>
 #include <netinet/tcp.h>
 
+/*
+ * Please keep as many data structures defined in .c as possible
+ * to increase forward ABI compatibility.
+ */
+
+
+struct tnt_connection
+{
+	/** The socket used to get connected to the server. */
+	int data_port;
+};
+
+
+/** A helper to get the DNS resolution done.
+ */
+
 static
 struct sockaddr_in
-get_sockaddr_in(const char *hostname, unsigned short port) {
-  struct sockaddr_in result;
+get_sockaddr_in(const char *hostname, unsigned short port)
+{
+	struct sockaddr_in result;
 
-  memset((void*)(&result), 0, sizeof(result));
-  result.sin_family = AF_INET;
-  result.sin_port   = htons(port);
+	memset((void*)(&result), 0, sizeof(result));
+	result.sin_family = AF_INET;
+	result.sin_port   = htons(port);
 
-  struct hostent *host = gethostbyname(hostname);
-  if (host != 0)
-	  memcpy((void*)(&result.sin_addr),
-		 (void*)(host->h_addr), host->h_length);
+	/* @todo: start using gethostbyname_r */
+	struct hostent *host = gethostbyname(hostname);
+	if (host != 0)
+		memcpy((void*)(&result.sin_addr),
+		       (void*)(host->h_addr), host->h_length);
 
-  return result;
+	return result;
 }
 
-struct tnt_connection *tnt_connect(const char *hostname, int port) {
-  int fd = socket(AF_INET, SOCK_STREAM, 0);
-  if (fd < 0)
-	  return NULL;
 
-  int opt = 1;
-  if (setsockopt(fd, IPPROTO_TCP, TCP_NODELAY, &opt, sizeof(opt)) == -1)
-	  return NULL;
+struct tnt_connection *tnt_connect(const char *hostname, int port)
+{
+	int fd = socket(AF_INET, SOCK_STREAM, 0);
+	if (fd < 0)
+		return NULL;
+
+	/*
+	 * We set TCP_NODELAY since we're not strictly
+	 * request/response.
+         */
+	int opt = 1;
+	if (setsockopt(fd, IPPROTO_TCP, TCP_NODELAY, &opt, sizeof(opt)) == -1)
+		return NULL;
 
-  struct sockaddr_in addr = get_sockaddr_in(hostname, port);
-  if (connect(fd, (struct sockaddr*)&addr, sizeof addr))
-	  return NULL;
+	struct sockaddr_in addr = get_sockaddr_in(hostname, port);
+	if (connect(fd, (struct sockaddr*)&addr, sizeof addr))
+		return NULL;
 
-  struct tnt_connection *conn = malloc(sizeof(struct tnt_connection));
-  conn->data_port = fd;
-  return conn;
+	struct tnt_connection *tnt = malloc(sizeof(struct tnt_connection));
+	if (tnt == NULL) {
+		close(fd);
+		return NULL;
+	}
+	tnt->data_port = fd;
+	return tnt;
 }
 
-void tnt_disconnect(struct tnt_connection *conn) {
-  close(conn->data_port);
-  free(conn);
+
+void tnt_disconnect(struct tnt_connection *tnt)
+{
+	close(tnt->data_port);
+	free(tnt);
 }
 
-int tnt_execute_raw(struct tnt_connection *conn, const char *message,
-		    size_t len) {
-  if (send(conn->data_port, message, len, 0) < 0)
-	  return 3;
 
-  char buf[2048];
-  if (recv(conn->data_port, buf, 2048, 0) < 16)
-	  return 3;
+/** Send the binary blob message to the server, read the response
+ * and learn as much from it as possible.
+ *
+ * @return 0 on success, 1 on error.
+ */
+
+int tnt_execute_raw(struct tnt_connection *tnt, const char *message,
+		    size_t len, struct tnt_result *tnt_res)
+{
+	if (send(tnt->data_port, message, len, 0) < 0)
+		return -1;
+
+	char buf[2048];
+
+	if (recv(tnt->data_port, buf, 2048, 0) < 16)
+		return -1;
+
+	if (tnt_res) {
+		memset(tnt_res, 0, sizeof *tnt_res);
+
+		int ret_code = buf[12];
+		int b = 256;
+		int i = 13;
+		while (i < 16) {
+			ret_code += (buf[i++] * b);
+			b *= 256;
+		}
 
-  return buf[12]; // return_code: 0,1,2
+		tnt_res->errcode = ret_code; /* see iproto.h */
+	}
+	return 0;
 }

connector/c/client.h

@@ -28,14 +28,24 @@
  */
 
 #include <stddef.h>
+#include <inttypes.h>
 
 /**
  * A connection with a Tarantool server.
  */
-struct tnt_connection {
-	int data_port;
+struct tnt_connection;
+
+
+/** Result of an operation, on an established
+ * connection.
+ */
+struct tnt_result
+{
+	/** Server error or 0. */
+	uint32_t errcode;
 };
 
+
 /**
  * Open a connection with a Tarantool server.
  *
@@ -46,24 +56,39 @@ struct tnt_connection {
  */
 struct tnt_connection *tnt_connect(const char *hostname, int port);
 
+
 /**
  * Close a connection.
  *
- * @param conn the connection.
+ * @param tnt the connection.
  */
-void tnt_disconnect(struct tnt_connection *conn);
+void tnt_disconnect(struct tnt_connection *tnt);
+
 
 /**
- * Execute a statement on a connection
+ * Execute a statement on a Tarantool server.
  *
- * @param conn the connection.
- * @param message a raw message following Tarantool protocol.
- * @param len the length of the message.
+ * @param       tnt     the connection.
+ * @param       message a raw message following Tarantool
+ * protocol.
+ * @param       len     the length of the message.
+ * @param[out]  tnt_res optional, can be NULL. If given,
+ * contains the server response.
  *
- * @return the status code: 0,1,2 if the statement was successufully
- * sent (see Tarantool protocol) or 3 if an error occured.
+ * @retval  0 if the statement was successfully sent (see Tarantool
+ * protocol) and a response from the server was successfully
+ * received.
+ * @retval  -1 if a client error occurred. Server error can be
+ * queried by looking into tnt_res.
+ */
+
+int tnt_execute_raw(struct tnt_connection *tnt, const char *message,
+		    size_t len, struct tnt_result *tnt_res);
+
+
+/** Return the *server* error code of the last error (see
+ * errcode.h), or 0 if there were no server error.
  */
-int tnt_execute_raw(struct tnt_connection *conn, const char *message,
-		    size_t len);
+uint32_t tnt_get_errcode(struct tnt_result *tnt_res);
 
 #endif /* TARANTOOL_CONNECTOR_CLIENT_H_INCLUDED */

include/errcode.h

@@ -14,7 +14,7 @@ struct errcode_record {
 #define ERROR_CODES(_)					    \
 	/*  0 */_(ERR_CODE_OK,				0, "OK") \
 	/*  1 */_(ERR_CODE_NONMASTER,			2, "Non master connection, but it should be") \
-	/*  2 */_(ERR_CODE_ILLEGAL_PARAMS,		2, "Illegal parametrs") \
+	/*  2 */_(ERR_CODE_ILLEGAL_PARAMS,		2, "Illegal parameters") \
 	/*  3 */_(ERR_CODE_BAD_UID,			2, "Uid is not from this storage range") \
 	/*  4 */_(ERR_CODE_NODE_IS_RO,			1, "Node is marked as read-only") \
 	/*  5 */_(ERR_CODE_NODE_IS_NOT_LOCKED,		1, "Node isn't locked") \

test/CMakeLists.txt

@@ -3,7 +3,8 @@ add_custom_target(test
     )
 
 add_executable(box/protocol
-  ${CMAKE_SOURCE_DIR}/test/box/protocol.c)
+    ${CMAKE_SOURCE_DIR}/test/box/protocol.c
+    ${CMAKE_SOURCE_DIR}/core/errcode.c)
 target_link_libraries (box/protocol client)
 
 install (PROGRAMS tarantool DESTINATION bin)

test/box/protocol.c

 #include <connector/c/client.h>
+#include <include/errcode.h>
 #include <stdio.h>
 
 /** Server connection. Reused between tests. */
@@ -12,7 +13,7 @@ void test_ping()
 		0x0, 0x0, 0x0, 0x0,    0x0, 0x0, 0x0, 0x0,
 		0x0, 0x0, 0x0, 0x0,    0x1, 0x0, 0x0, 0x0,
 		0x4, 0x1, 0x0, 0x0, 0x0 };
-	int res = tnt_execute_raw(conn, message, sizeof message);
+	int res = tnt_execute_raw(conn, message, sizeof message, 0);
 	printf("return_code: %d\n", res); /* =0 */
 }
 
@@ -28,8 +29,11 @@ void test_bug702397()
 		0x11, 0x0, 0x0, 0x0,    0x14, 0x0, 0x0, 0x0,    0x0, 0x0, 0x0, 0x0,
 		0x0, 0x0, 0x0, 0x0,     0x0, 0x0, 0x0, 0x0,     0x0, 0x0, 0x0, 0x0,
 		0xff, 0xff, 0xff, 0xff, 0x0, 0x0, 0x0, 0x0 };
-	int res = tnt_execute_raw(conn, message, sizeof message);
-	printf("return_code: %d\n", res); // =2
+	struct tnt_result tnt_res;
+	int res = tnt_execute_raw(conn, message, sizeof message, &tnt_res);
+	printf("return_code: %s, %s\n",
+	       tnt_errcode_str(tnt_res.errcode >> 8),
+	       tnt_errcode_desc(tnt_res.errcode >> 8));
 }
 
 int main()
@@ -42,5 +46,5 @@ int main()
 	test_bug702397();
 
 	tnt_disconnect(conn);
-  return 0;
+	return 0;
 }

test/box/protocol.result

 return_code: 0
-return_code: 2
+return_code: ERR_CODE_ILLEGAL_PARAMS, "Illegal parameters"