From e9fe6aad66010614f987e917866837706d19053b Mon Sep 17 00:00:00 2001
From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Date: Tue, 15 Dec 2020 09:21:10 -0500
Subject: [PATCH] Cleanup: clarify ownership of excluder

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: I3947721841222859f4f4eab15ae94856921eb09b
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
---
 liblttng-ust/lttng-events.c        | 12 +++++++-----
 liblttng-ust/lttng-ust-abi.c       |  4 ++--
 liblttng-ust/lttng-ust-comm.c      | 13 ++++---------
 liblttng-ust/ust-events-internal.h |  4 ++--
 4 files changed, 15 insertions(+), 18 deletions(-)

diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c
index 16d75da9..fbebc338 100644
--- a/liblttng-ust/lttng-events.c
+++ b/liblttng-ust/lttng-events.c
@@ -1474,14 +1474,16 @@ int lttng_event_enabler_attach_filter_bytecode(struct lttng_event_enabler *event
 
 static
 void _lttng_enabler_attach_exclusion(struct lttng_enabler *enabler,
-		struct lttng_ust_excluder_node *excluder)
+		struct lttng_ust_excluder_node **excluder)
 {
-	excluder->enabler = enabler;
-	cds_list_add_tail(&excluder->node, &enabler->excluder_head);
+	(*excluder)->enabler = enabler;
+	cds_list_add_tail(&(*excluder)->node, &enabler->excluder_head);
+	/* Take ownership of excluder */
+	*excluder = NULL;
 }
 
 int lttng_event_enabler_attach_exclusion(struct lttng_event_enabler *event_enabler,
-		struct lttng_ust_excluder_node *excluder)
+		struct lttng_ust_excluder_node **excluder)
 {
 	_lttng_enabler_attach_exclusion(
 		lttng_event_enabler_as_enabler(event_enabler), excluder);
@@ -1536,7 +1538,7 @@ int lttng_event_notifier_enabler_attach_capture_bytecode(
 
 int lttng_event_notifier_enabler_attach_exclusion(
 		struct lttng_event_notifier_enabler *event_notifier_enabler,
-		struct lttng_ust_excluder_node *excluder)
+		struct lttng_ust_excluder_node **excluder)
 {
 	_lttng_enabler_attach_exclusion(
 		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 7490f8b7..5063ec6c 100644
--- a/liblttng-ust/lttng-ust-abi.c
+++ b/liblttng-ust/lttng-ust-abi.c
@@ -715,7 +715,7 @@ long lttng_event_notifier_enabler_cmd(int objd, unsigned int cmd, unsigned long
 			(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);
+			(struct lttng_ust_excluder_node **) arg);
 	case LTTNG_UST_CAPTURE:
 		return lttng_event_notifier_enabler_attach_capture_bytecode(
 			event_notifier_enabler,
@@ -1299,7 +1299,7 @@ long lttng_event_enabler_cmd(int objd, unsigned int cmd, unsigned long arg,
 	case LTTNG_UST_EXCLUSION:
 	{
 		return lttng_event_enabler_attach_exclusion(enabler,
-				(struct lttng_ust_excluder_node *) arg);
+				(struct lttng_ust_excluder_node **) arg);
 	}
 	default:
 		return -EINVAL;
diff --git a/liblttng-ust/lttng-ust-comm.c b/liblttng-ust/lttng-ust-comm.c
index 7e3bf450..69bce955 100644
--- a/liblttng-ust/lttng-ust-comm.c
+++ b/liblttng-ust/lttng-ust-comm.c
@@ -971,18 +971,13 @@ int handle_message(struct sock_info *sock_info,
 				goto error;
 			}
 		}
-		if (ops->cmd) {
+		if (ops->cmd)
 			ret = ops->cmd(lum->handle, lum->cmd,
-					(unsigned long) node,
+					(unsigned long) &node,
 					&args, sock_info);
-			if (ret) {
-				free(node);
-			}
-			/* Don't free exclusion data if everything went fine. */
-		} else {
+		else
 			ret = -ENOSYS;
-			free(node);
-		}
+		free(node);
 		break;
 	}
 	case LTTNG_UST_EVENT_NOTIFIER_GROUP_CREATE:
diff --git a/liblttng-ust/ust-events-internal.h b/liblttng-ust/ust-events-internal.h
index be5c549c..73cc22bf 100644
--- a/liblttng-ust/ust-events-internal.h
+++ b/liblttng-ust/ust-events-internal.h
@@ -151,7 +151,7 @@ int lttng_event_enabler_attach_context(struct lttng_event_enabler *enabler,
  */
 LTTNG_HIDDEN
 int lttng_event_enabler_attach_exclusion(struct lttng_event_enabler *enabler,
-		struct lttng_ust_excluder_node *excluder);
+		struct lttng_ust_excluder_node **excluder);
 
 /*
  * Synchronize bytecodes for the enabler and the instance (event or
@@ -242,7 +242,7 @@ int lttng_event_notifier_enabler_attach_capture_bytecode(
 LTTNG_HIDDEN
 int lttng_event_notifier_enabler_attach_exclusion(
 		struct lttng_event_notifier_enabler *event_notifier_enabler,
-		struct lttng_ust_excluder_node *excluder);
+		struct lttng_ust_excluder_node **excluder);
 
 LTTNG_HIDDEN
 void lttng_free_event_notifier_filter_runtime(
-- 
2.34.1