From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Date: Tue, 15 Dec 2020 14:24:30 +0000 (-0500)
Subject: Cleanup: clarify bytecode ownership
X-Git-Tag: v2.13.0-rc1~391
X-Git-Url: http://git.lttng.org/?p=lttng-ust.git;a=commitdiff_plain;h=ab89263ef2f50399e608e74557900aa0a3165a61

Cleanup: clarify bytecode ownership

Use a regular pattern for all commands:

If the command callback takes ownership of a pointer or file descriptor,
it sets them to NULL or -1. Therefore, the caller can always try to free
the pointer, or close it if it is greater or equal to 0.

Change-Id: Ie155d3cff80930271417075d693aa4528decbfc3
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
---

diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c
index fbebc338..664f8b5d 100644
--- a/liblttng-ust/lttng-events.c
+++ b/liblttng-ust/lttng-events.c
@@ -1456,14 +1456,16 @@ int lttng_event_enabler_disable(struct lttng_event_enabler *event_enabler)
 
 static
 void _lttng_enabler_attach_filter_bytecode(struct lttng_enabler *enabler,
-		struct lttng_ust_bytecode_node *bytecode)
+		struct lttng_ust_bytecode_node **bytecode)
 {
-	bytecode->enabler = enabler;
-	cds_list_add_tail(&bytecode->node, &enabler->filter_bytecode_head);
+	(*bytecode)->enabler = enabler;
+	cds_list_add_tail(&(*bytecode)->node, &enabler->filter_bytecode_head);
+	/* Take ownership of bytecode */
+	*bytecode = NULL;
 }
 
 int lttng_event_enabler_attach_filter_bytecode(struct lttng_event_enabler *event_enabler,
-		struct lttng_ust_bytecode_node *bytecode)
+		struct lttng_ust_bytecode_node **bytecode)
 {
 	_lttng_enabler_attach_filter_bytecode(
 		lttng_event_enabler_as_enabler(event_enabler), bytecode);
@@ -1512,7 +1514,7 @@ int lttng_event_notifier_enabler_disable(
 
 int lttng_event_notifier_enabler_attach_filter_bytecode(
 		struct lttng_event_notifier_enabler *event_notifier_enabler,
-		struct lttng_ust_bytecode_node *bytecode)
+		struct lttng_ust_bytecode_node **bytecode)
 {
 	_lttng_enabler_attach_filter_bytecode(
 		lttng_event_notifier_enabler_as_enabler(event_notifier_enabler),
diff --git a/liblttng-ust/lttng-ust-abi.c b/liblttng-ust/lttng-ust-abi.c
index 5063ec6c..9ba5acd5 100644
--- a/liblttng-ust/lttng-ust-abi.c
+++ b/liblttng-ust/lttng-ust-abi.c
@@ -712,7 +712,7 @@ long lttng_event_notifier_enabler_cmd(int objd, unsigned int cmd, unsigned long
 	case LTTNG_UST_FILTER:
 		return lttng_event_notifier_enabler_attach_filter_bytecode(
 			event_notifier_enabler,
-			(struct lttng_ust_bytecode_node *) arg);
+			(struct lttng_ust_bytecode_node **) arg);
 	case LTTNG_UST_EXCLUSION:
 		return lttng_event_notifier_enabler_attach_exclusion(event_notifier_enabler,
 			(struct lttng_ust_excluder_node **) arg);
@@ -1291,7 +1291,7 @@ long lttng_event_enabler_cmd(int objd, unsigned int cmd, unsigned long arg,
 		int ret;
 
 		ret = lttng_event_enabler_attach_filter_bytecode(enabler,
-				(struct lttng_ust_bytecode_node *) arg);
+				(struct lttng_ust_bytecode_node **) arg);
 		if (ret)
 			return ret;
 		return 0;
diff --git a/liblttng-ust/lttng-ust-comm.c b/liblttng-ust/lttng-ust-comm.c
index 69bce955..682992ca 100644
--- a/liblttng-ust/lttng-ust-comm.c
+++ b/liblttng-ust/lttng-ust-comm.c
@@ -772,7 +772,7 @@ static
 int handle_bytecode_recv(struct sock_info *sock_info,
 		int sock, struct ustcomm_ust_msg *lum)
 {
-	struct lttng_ust_bytecode_node *bytecode;
+	struct lttng_ust_bytecode_node *bytecode = NULL;
 	enum lttng_ust_bytecode_node_type type;
 	const struct lttng_ust_objd_ops *ops;
 	uint32_t data_size, data_size_max, reloc_offset;
@@ -829,7 +829,7 @@ int handle_bytecode_recv(struct sock_info *sock_info,
 	switch (len) {
 	case 0:	/* orderly shutdown */
 		ret = 0;
-		goto error_free_bytecode;
+		goto end;
 	default:
 		if (len == bytecode->bc.len) {
 			DBG("Bytecode %s data received",
@@ -842,41 +842,33 @@ int handle_bytecode_recv(struct sock_info *sock_info,
 				ERR("%s remote end closed connection",
 						sock_info->name);
 				ret = len;
-				goto error_free_bytecode;
+				goto end;
 			}
 			ret = len;
-			goto error_free_bytecode;
+			goto end;
 		} else {
 			DBG("Incorrect %s bytecode data message size: %zd",
 					bytecode_type_str(lum->cmd), len);
 			ret = -EINVAL;
-			goto error_free_bytecode;
+			goto end;
 		}
 	}
 
 	ops = objd_ops(lum->handle);
 	if (!ops) {
 		ret = -ENOENT;
-		goto error_free_bytecode;
+		goto end;
 	}
 
-	if (ops->cmd) {
+	if (ops->cmd)
 		ret = ops->cmd(lum->handle, lum->cmd,
-			(unsigned long) bytecode,
+			(unsigned long) &bytecode,
 			NULL, sock_info);
-		if (ret)
-			goto error_free_bytecode;
-		/* don't free bytecode if everything went fine. */
-	} else {
+	else
 		ret = -ENOSYS;
-		goto error_free_bytecode;
-	}
-
-	goto end;
 
-error_free_bytecode:
-	free(bytecode);
 end:
+	free(bytecode);
 	return ret;
 }
 
diff --git a/liblttng-ust/ust-events-internal.h b/liblttng-ust/ust-events-internal.h
index 73cc22bf..e1141add 100644
--- a/liblttng-ust/ust-events-internal.h
+++ b/liblttng-ust/ust-events-internal.h
@@ -134,7 +134,7 @@ int lttng_event_enabler_disable(struct lttng_event_enabler *enabler);
 LTTNG_HIDDEN
 int lttng_event_enabler_attach_filter_bytecode(
 		struct lttng_event_enabler *enabler,
-		struct lttng_ust_bytecode_node *bytecode);
+		struct lttng_ust_bytecode_node **bytecode);
 
 /*
  * Attach an application context to an event enabler.
@@ -224,7 +224,7 @@ int lttng_event_notifier_enabler_disable(
 LTTNG_HIDDEN
 int lttng_event_notifier_enabler_attach_filter_bytecode(
 		struct lttng_event_notifier_enabler *event_notifier_enabler,
-		struct lttng_ust_bytecode_node *bytecode);
+		struct lttng_ust_bytecode_node **bytecode);
 
 /*
  * Attach capture bytecode program to `struct lttng_event_notifier_enabler` and