From: Andre Noll <maan@tuebingen.mpg.de>
Date: Sat, 4 Apr 2015 23:20:00 +0000 (+0000)
Subject: afs: Pass sideband error packet on callback failures.
X-Git-Tag: v0.5.6~94^2~27
X-Git-Url: http://git.tuebingen.mpg.de/?p=paraslash.git;a=commitdiff_plain;h=586e2d24be69bc0aca8ad6c2033a4f4d193f5372;ds=sidebyside

afs: Pass sideband error packet on callback failures.

This changes the afs callback mechanism to honor negative return
values from a callback. We now send a special "callback failure"
sideband packet to the command handler in this case. This packet
contains the (negative) return value of the callback.

The dispatcher for afs callback results reads the error code and passes
it back via the callback request functions to the caller of the command
handler in handle_connect(). The latter already does the right thing:
It translates the error code into a string and sends this string to
the client.

This commit changes the callback of the ls command to return negative
on errors. With the patch applied the command

	para_client ls /does/not/exist

now exits with status 1.

Other afs commands will make use of the new feature in subsequent
commits.
---

diff --git a/afs.c b/afs.c
index 24e737b6..4bc7871c 100644
--- a/afs.c
+++ b/afs.c
@@ -166,13 +166,8 @@ static int dispatch_result(int result_shmid, callback_result_handler *handler,
 	}
 	result.size = cr->result_size;
 	result.data = result_shm + sizeof(*cr);
-	if (result.size) {
-		assert(handler);
-		ret = handler(&result, cr->band, private_result_data);
-		if (ret < 0)
-			PARA_NOTICE_LOG("result handler error: %s\n",
-				para_strerror(-ret));
-	}
+	assert(handler);
+	ret = handler(&result, cr->band, private_result_data);
 	ret2 = shm_detach(result_shm);
 	if (ret2 < 0) {
 		PARA_ERROR_LOG("detach failed: %s\n", para_strerror(-ret2));
@@ -261,12 +256,10 @@ int send_callback_request(callback_function *f, struct osl_object *query,
 		ret = *(int *) buf;
 		assert(ret > 0);
 		result_shmid = ret;
-		if (!dispatch_error) {
-			ret = dispatch_result(result_shmid, result_handler,
-				private_result_data);
-			if (ret < 0)
-				dispatch_error = 1;
-		}
+		ret = dispatch_result(result_shmid, result_handler,
+			private_result_data);
+		if (ret < 0 && dispatch_error >= 0)
+			dispatch_error = ret;
 		ret = shm_destroy(result_shmid);
 		if (ret < 0)
 			PARA_CRIT_LOG("destroy result failed: %s\n",
@@ -278,8 +271,11 @@ out:
 		PARA_CRIT_LOG("shm destroy error\n");
 	if (fd >= 0)
 		close(fd);
-//	PARA_DEBUG_LOG("callback_ret: %d\n", ret);
-	return ret < 0? ret : num_dispatched;
+	if (dispatch_error < 0)
+		return dispatch_error;
+	if (ret < 0)
+		return ret;
+	return num_dispatched;
 }
 
 /**
@@ -559,9 +555,22 @@ int afs_cb_result_handler(struct osl_object *result, uint8_t band,
 	struct command_context *cc = private;
 
 	assert(cc);
-	if (!result->size)
-		return 1;
-	return send_sb(&cc->scc, result->data, result->size, band, true);
+	switch (band) {
+	case SBD_OUTPUT:
+	case SBD_DEBUG_LOG:
+	case SBD_INFO_LOG:
+	case SBD_NOTICE_LOG:
+	case SBD_WARNING_LOG:
+	case SBD_ERROR_LOG:
+	case SBD_CRIT_LOG:
+	case SBD_EMERG_LOG:
+		assert(result->size > 0);
+		return send_sb(&cc->scc, result->data, result->size, band, true);
+	case SBD_AFS_CB_FAILURE:
+		return *(int *)(result->data);
+	default:
+		return -E_BAD_BAND;
+	}
 }
 
 void flush_and_free_pb(struct para_buffer *pb)
@@ -808,8 +817,8 @@ int pass_buffer_as_shm(int fd, uint8_t band, const char *buf, size_t size)
 	void *shm;
 	struct callback_result *cr;
 
-	if (!buf || !size)
-		return 0;
+	if (size == 0)
+		assert(band != SBD_OUTPUT);
 	ret = shm_new(size + sizeof(*cr));
 	if (ret < 0)
 		return ret;
@@ -820,7 +829,8 @@ int pass_buffer_as_shm(int fd, uint8_t band, const char *buf, size_t size)
 	cr = shm;
 	cr->result_size = size;
 	cr->band = band;
-	memcpy(shm + sizeof(*cr), buf, size);
+	if (size > 0)
+		memcpy(shm + sizeof(*cr), buf, size);
 	ret = shm_detach(shm);
 	if (ret < 0)
 		goto err;
@@ -838,7 +848,7 @@ static int call_callback(int fd, int query_shmid)
 	void *query_shm;
 	struct callback_query *cq;
 	struct osl_object query;
-	int ret;
+	int ret, ret2;
 
 	ret = shm_attach(query_shmid, ATTACH_RW, &query_shm);
 	if (ret < 0)
@@ -846,8 +856,23 @@ static int call_callback(int fd, int query_shmid)
 	cq = query_shm;
 	query.data = (char *)query_shm + sizeof(*cq);
 	query.size = cq->query_size;
-	cq->handler(fd, &query);
-	return shm_detach(query_shm);
+	ret = cq->handler(fd, &query);
+	ret2 = shm_detach(query_shm);
+	if (ret2 < 0) {
+		if (ret < 0) /* ignore (but log) detach error */
+			PARA_ERROR_LOG("could not detach sma: %s\n",
+				para_strerror(-ret2));
+		else
+			ret = ret2;
+	}
+	if (ret < 0) {
+		ret2 = pass_buffer_as_shm(fd, SBD_AFS_CB_FAILURE,
+			(const char *)&ret, sizeof(ret));
+		if (ret2 < 0)
+			PARA_ERROR_LOG("could not pass cb failure packet: %s\n",
+				para_strerror(-ret));
+	}
+	return ret;
 }
 
 static int execute_server_command(fd_set *rfds)
diff --git a/aft.c b/aft.c
index acb6c6bf..6b5e0b74 100644
--- a/aft.c
+++ b/aft.c
@@ -1323,8 +1323,7 @@ static int com_ls_callback(int fd, const struct osl_object *query)
 	if (ret < 0)
 		goto out;
 	if (opts->num_matching_paths == 0) {
-		if (opts->num_patterns > 0)
-			para_printf(&b, "no matches\n");
+		ret = opts->num_patterns > 0? -E_NO_MATCH : 0;
 		goto out;
 	}
 	ret = sort_matching_paths(opts);
@@ -1348,7 +1347,7 @@ out:
 	free(opts->data);
 	free(opts->data_ptr);
 	free(opts->patterns);
-	return 0;
+	return ret;
 }
 
 /*
@@ -1356,7 +1355,7 @@ out:
  */
 int com_ls(struct command_context *cc)
 {
-	int i, ret;
+	int i;
 	unsigned flags = 0;
 	enum ls_sorting_method sort = LS_SORT_BY_PATH;
 	enum ls_listing_mode mode = LS_MODE_SHORT;
@@ -1465,9 +1464,8 @@ int com_ls(struct command_context *cc)
 	opts.sorting = sort;
 	opts.mode = mode;
 	opts.num_patterns = cc->argc - i;
-	ret = send_option_arg_callback_request(&query, opts.num_patterns,
+	return send_option_arg_callback_request(&query, opts.num_patterns,
 		cc->argv + i, com_ls_callback, afs_cb_result_handler, cc);
-	return ret;
 }
 
 /**
@@ -1777,7 +1775,9 @@ static int get_row_pointer_from_result(struct osl_object *result,
 		__a_unused uint8_t band, void *private)
 {
 	struct osl_row **row = private;
-	*row = *(struct osl_row **)(result->data);
+
+	if (band == SBD_OUTPUT)
+		*row = *(struct osl_row **)(result->data);
 	return 1;
 }
 
diff --git a/error.h b/error.h
index 28ac9552..96a0c9a3 100644
--- a/error.h
+++ b/error.h
@@ -275,6 +275,7 @@ extern const char **para_errlist[];
 	PARA_ERROR(NO_AFHI, "audio format handler info required"), \
 	PARA_ERROR(AFT_SYNTAX, "audio file table syntax error"), \
 	PARA_ERROR(HASH_MISMATCH, "hash mismatch, consider re-add"), \
+	PARA_ERROR(NO_MATCH, "no matches"), \
 
 
 #define USER_LIST_ERRORS \
diff --git a/sideband.h b/sideband.h
index 9ce0bb61..c9b698f9 100644
--- a/sideband.h
+++ b/sideband.h
@@ -60,6 +60,8 @@
 	DESIGNATOR(EXIT__FAILURE), \
 	/* The next chunk of the blob (addblob commands only) */ \
 	DESIGNATOR(BLOB_DATA), \
+	/* An afs callback failed. */ \
+	DESIGNATOR(AFS_CB_FAILURE), \
 
 /** Just prefix with \p SBD_. */
 #define DESIGNATOR(x) SBD_ ## x