From: Andre Noll <maan@tuebingen.mpg.de>
Date: Thu, 9 Apr 2015 12:51:49 +0000 (+0000)
Subject: Let afs_event() return int.
X-Git-Tag: v0.5.6~94^2~7
X-Git-Url: http://git.tuebingen.mpg.de/?p=paraslash.git;a=commitdiff_plain;h=fe282244d61b83eb1f285259dd1d55992a96288f;ds=sidebyside

Let afs_event() return int.

It is usually a critical error if an afs event handler returns an
error. Currently we only print a log message an continue in this
case. The callers of afs_event() which trigger the event have no way
to tell that something went wrong, since this function returns void.

By returning int instead of void the callers can abort in the error
case. Most of the callers are afs callbacks. These can propagate the
error code to the command handler process, which will translate the
error code into a string and send it to the client.  All callbacks
are changed in this way.
---

diff --git a/afs.c b/afs.c
index 0c615747..67b473a4 100644
--- a/afs.c
+++ b/afs.c
@@ -1170,10 +1170,14 @@ int com_check(struct command_context *cc)
  * \param pb May be \p NULL.
  * \param data Type depends on \a event.
  *
- * This function calls the table handlers of all tables and passes \a pb and \a
- * data verbatim. It's up to the handlers to interpret the \a data pointer.
+ * This function calls each table event handler, passing \a pb and \a data
+ * verbatim. It's up to the handlers to interpret the \a data pointer. If a
+ * handler returns negative, the loop is aborted.
+ *
+ * \return The (negative) error code of the first handler that failed, or non-negative
+ * if all handlers succeeded.
  */
-void afs_event(enum afs_events event, struct para_buffer *pb,
+__must_check int afs_event(enum afs_events event, struct para_buffer *pb,
 		void *data)
 {
 	int i, ret;
@@ -1183,10 +1187,13 @@ void afs_event(enum afs_events event, struct para_buffer *pb,
 		if (!t->event_handler)
 			continue;
 		ret = t->event_handler(event, pb, data);
-		if (ret < 0)
+		if (ret < 0) {
 			PARA_CRIT_LOG("table %s, event %d: %s\n", t->name,
 				event, para_strerror(-ret));
+			return ret;
+		}
 	}
+	return 1;
 }
 
 /**
diff --git a/afs.h b/afs.h
index efd7fbac..8fd8313a 100644
--- a/afs.h
+++ b/afs.h
@@ -209,7 +209,7 @@ _static_inline_ int afs_max_size_handler(char *buf, size_t size, void *private)
 }
 
 __noreturn void afs_init(uint32_t cookie, int socket_fd);
-void afs_event(enum afs_events event, struct para_buffer *pb,
+__must_check int afs_event(enum afs_events event, struct para_buffer *pb,
 	void *data);
 int send_callback_request(callback_function *f, struct osl_object *query,
 		callback_result_handler *result_handler,
diff --git a/aft.c b/aft.c
index 0207705f..9f6c4e29 100644
--- a/aft.c
+++ b/aft.c
@@ -1072,7 +1072,9 @@ again:
 	 * No need to update the status items as the AFSI_CHANGE event will
 	 * recreate them.
 	 */
-	afs_event(AFSI_CHANGE, NULL, &aced);
+	ret = afs_event(AFSI_CHANGE, NULL, &aced);
+	if (ret < 0)
+		goto out;
 	ret = save_afd(afd);
 out:
 	free(afd->afhi.chunk_table);
@@ -1658,7 +1660,9 @@ static int com_add_callback(int fd, const struct osl_object *query)
 		if (pb) { /* hs trumps pb, remove pb */
 			if (flags & ADD_FLAG_VERBOSE)
 				para_printf(&msg, "removing %s\n", path);
-			afs_event(AUDIO_FILE_REMOVE, &msg, pb);
+			ret = afs_event(AUDIO_FILE_REMOVE, &msg, pb);
+			if (ret < 0)
+				goto out;
 			ret = osl(osl_del_row(audio_file_table, pb));
 			if (ret < 0)
 				goto out;
@@ -1676,7 +1680,9 @@ static int com_add_callback(int fd, const struct osl_object *query)
 			&objs[AFTCOL_PATH]));
 		if (ret < 0)
 			goto out;
-		afs_event(AUDIO_FILE_RENAME, &msg, hs);
+		ret = afs_event(AUDIO_FILE_RENAME, &msg, hs);
+		if (ret < 0)
+			goto out;
 		if (!(flags & ADD_FLAG_FORCE))
 			goto out;
 	}
@@ -1719,7 +1725,9 @@ static int com_add_callback(int fd, const struct osl_object *query)
 			&objs[AFTCOL_CHUNKS]));
 		if (ret < 0)
 			goto out;
-		afs_event(AFHI_CHANGE, &msg, row);
+		ret = afs_event(AFHI_CHANGE, &msg, row);
+		if (ret < 0)
+			goto out;
 		goto out;
 	}
 	/* new entry, use default afsi */
@@ -1732,7 +1740,7 @@ static int com_add_callback(int fd, const struct osl_object *query)
 	objs[AFTCOL_AFSI].size = AFSI_SIZE;
 	save_afsi(&default_afsi, &objs[AFTCOL_AFSI]);
 	ret = osl(osl_add_and_get_row(audio_file_table, objs, &aft_row));
-	afs_event(AUDIO_FILE_ADD, &msg, aft_row);
+	ret = afs_event(AUDIO_FILE_ADD, &msg, aft_row);
 out:
 	if (ret < 0)
 		para_printf(&msg, "could not add %s\n", path);
@@ -2007,8 +2015,7 @@ static int touch_audio_file(__a_unused struct osl_table *table,
 	save_afsi(&new_afsi, &obj); /* in-place update */
 	aced.aft_row = row;
 	aced.old_afsi = &old_afsi;
-	afs_event(AFSI_CHANGE, &tad->pb, &aced);
-	return 1;
+	return afs_event(AFSI_CHANGE, &tad->pb, &aced);
 }
 
 static int com_touch_callback(int fd, const struct osl_object *query)
@@ -2139,7 +2146,9 @@ static int remove_audio_file(__a_unused struct osl_table *table,
 
 	if (crd->flags & RM_FLAG_VERBOSE)
 		para_printf(&crd->pb, "removing %s\n", name);
-	afs_event(AUDIO_FILE_REMOVE, &crd->pb, row);
+	ret = afs_event(AUDIO_FILE_REMOVE, &crd->pb, row);
+	if (ret < 0)
+		return ret;
 	ret = osl(osl_del_row(audio_file_table, row));
 	if (ret < 0)
 		para_printf(&crd->pb, "cannot remove %s\n", name);
@@ -2276,8 +2285,7 @@ static int copy_selector_info(__a_unused struct osl_table *table,
 		para_printf(&cad->pb, "copied afsi to %s\n", name);
 	aced.aft_row = row;
 	aced.old_afsi = &old_afsi;
-	afs_event(AFSI_CHANGE, &cad->pb, &aced);
-	return 1;
+	return afs_event(AFSI_CHANGE, &cad->pb, &aced);
 }
 
 static int com_cpsi_callback(int fd, const struct osl_object *query)
@@ -2398,8 +2406,7 @@ static int change_atts(__a_unused struct osl_table *table,
 	new_afsi.attributes |= cad->add_mask;
 	new_afsi.attributes &= ~cad->del_mask;
 	save_afsi(&new_afsi, &obj); /* in-place update */
-	afs_event(AFSI_CHANGE, &cad->pb, &aced);
-	return 1;
+	return afs_event(AFSI_CHANGE, &cad->pb, &aced);
 }
 
 static int com_setatt_callback(int fd, const struct osl_object *query)
diff --git a/attribute.c b/attribute.c
index af1400c8..5bb14ed4 100644
--- a/attribute.c
+++ b/attribute.c
@@ -280,7 +280,9 @@ static int com_addatt_callback(int fd, const struct osl_object *query)
 			goto out;
 		aed.name = p;
 		aed.bitnum = bitnum;
-		afs_event(ATTRIBUTE_ADD, &pb, &aed);
+		ret = afs_event(ATTRIBUTE_ADD, &pb, &aed);
+		if (ret < 0)
+			goto out;
 		greatest_att_bitnum = PARA_MAX(greatest_att_bitnum, (int)bitnum);
 	}
 out:
@@ -330,7 +332,7 @@ out:
 	if (ret < 0)
 		para_printf(&pb, "cannot rename %s to %s\n", old, new);
 	else
-		afs_event(ATTRIBUTE_RENAME, &pb, NULL);
+		ret = afs_event(ATTRIBUTE_RENAME, &pb, NULL);
 	flush_and_free_pb(&pb);
 	return ret;
 }
@@ -361,8 +363,7 @@ static int remove_attribute(struct osl_table *table, struct osl_row *row,
 		para_printf(pb, "cannot remove %s\n", name);
 		return ret;
 	}
-	afs_event(ATTRIBUTE_REMOVE, pb, &red);
-	return 1;
+	return afs_event(ATTRIBUTE_REMOVE, pb, &red);
 }
 
 static int com_rmatt_callback(int fd, const struct osl_object *query)
diff --git a/blob.c b/blob.c
index fbfea00c..1360963e 100644
--- a/blob.c
+++ b/blob.c
@@ -289,7 +289,7 @@ static int com_rmblob_callback(struct osl_table *table, int fd,
 		ret = -E_NO_MATCH;
 	else {
 		para_printf(&pb, "removed %d blob(s)\n", pmd.num_matches);
-		afs_event(BLOB_RENAME, NULL, table);
+		ret = afs_event(BLOB_RENAME, NULL, table);
 	}
 out:
 	flush_and_free_pb(&pb);
@@ -371,7 +371,7 @@ static int com_addblob_callback(struct osl_table *table, int fd,
 	ret = osl(osl_add_row(table, objs));
 	if (ret < 0)
 		goto out;
-	afs_event(BLOB_ADD, NULL, table);
+	ret = afs_event(BLOB_ADD, NULL, table);
 out:
 	if (ret < 0)
 		msg_len = xasprintf(&msg, "cannot add %s\n", name);
@@ -489,7 +489,7 @@ static int com_mvblob_callback(struct osl_table *table, int fd,
 		para_printf(&pb, "cannot rename blob %s to %s\n", src, dest);
 		goto out;
 	}
-	afs_event(BLOB_RENAME, NULL, table);
+	ret = afs_event(BLOB_RENAME, NULL, table);
 out:
 	flush_and_free_pb(&pb);
 	return ret;