From: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Date: Fri, 25 Mar 2022 19:43:56 +0000 (-0400)
Subject: Clean-up: sessiond: cmd_enable_channel_internal
X-Git-Url: https://git.lttng.org./?a=commitdiff_plain;h=4878de5c7deb512bbdac4fdfc498907efa06fb7c;p=lttng-tools.git

Clean-up: sessiond: cmd_enable_channel_internal

After catching an error code mixup in cmd_enable_channel_internal, this
change explicitly sets the return type of cmd_enable_channel_internal
and of its callees to `enum lttng_error_code` since those already return
these values as integers.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ic3c042c2ee3d259cc694e6aaf3a1a2f3ca843042
---

diff --git a/src/bin/lttng-sessiond/channel.cpp b/src/bin/lttng-sessiond/channel.cpp
index 9ad71ca16..c5b944d61 100644
--- a/src/bin/lttng-sessiond/channel.cpp
+++ b/src/bin/lttng-sessiond/channel.cpp
@@ -173,29 +173,28 @@ error:
 /*
  * Enable kernel channel of the kernel session.
  */
-int channel_kernel_enable(struct ltt_kernel_session *ksession,
+enum lttng_error_code channel_kernel_enable(struct ltt_kernel_session *ksession,
 		struct ltt_kernel_channel *kchan)
 {
-	int ret;
+	enum lttng_error_code ret_code;
 
 	LTTNG_ASSERT(ksession);
 	LTTNG_ASSERT(kchan);
 
 	if (kchan->enabled == 0) {
-		ret = kernel_enable_channel(kchan);
-		if (ret < 0) {
-			ret = LTTNG_ERR_KERN_CHAN_ENABLE_FAIL;
+		if (kernel_enable_channel(kchan) < 0) {
+			ret_code = LTTNG_ERR_KERN_CHAN_ENABLE_FAIL;
 			goto error;
 		}
 	} else {
-		ret = LTTNG_ERR_KERN_CHAN_EXIST;
+		ret_code = LTTNG_ERR_KERN_CHAN_EXIST;
 		goto error;
 	}
 
-	ret = LTTNG_OK;
+	ret_code = LTTNG_OK;
 
 error:
-	return ret;
+	return ret_code;
 }
 
 static int channel_validate(struct lttng_channel *attr)
@@ -223,10 +222,10 @@ static int channel_validate_kernel(struct lttng_channel *attr)
 /*
  * Create kernel channel of the kernel session and notify kernel thread.
  */
-int channel_kernel_create(struct ltt_kernel_session *ksession,
+enum lttng_error_code channel_kernel_create(struct ltt_kernel_session *ksession,
 		struct lttng_channel *attr, int kernel_pipe)
 {
-	int ret;
+	enum lttng_error_code ret_code;
 	struct lttng_channel *defattr = NULL;
 
 	LTTNG_ASSERT(ksession);
@@ -236,7 +235,7 @@ int channel_kernel_create(struct ltt_kernel_session *ksession,
 		defattr = channel_new_default_attr(LTTNG_DOMAIN_KERNEL,
 				LTTNG_BUFFER_GLOBAL);
 		if (defattr == NULL) {
-			ret = LTTNG_ERR_FATAL;
+			ret_code = LTTNG_ERR_FATAL;
 			goto error;
 		}
 		attr = defattr;
@@ -252,42 +251,40 @@ int channel_kernel_create(struct ltt_kernel_session *ksession,
 
 	/* Validate common channel properties. */
 	if (channel_validate(attr) < 0) {
-		ret = LTTNG_ERR_INVALID;
+		ret_code = LTTNG_ERR_INVALID;
 		goto error;
 	}
 
 	if (channel_validate_kernel(attr) < 0) {
-		ret = LTTNG_ERR_INVALID;
+		ret_code = LTTNG_ERR_INVALID;
 		goto error;
 	}
 
-	/* Channel not found, creating it */
-	ret = kernel_create_channel(ksession, attr);
-	if (ret < 0) {
-		ret = LTTNG_ERR_KERN_CHAN_FAIL;
+	/* Channel not found, creating it. */
+	if (kernel_create_channel(ksession, attr) < 0) {
+		ret_code = LTTNG_ERR_KERN_CHAN_FAIL;
 		goto error;
 	}
 
 	/* Notify kernel thread that there is a new channel */
-	ret = notify_thread_pipe(kernel_pipe);
-	if (ret < 0) {
-		ret = LTTNG_ERR_FATAL;
+	if (notify_thread_pipe(kernel_pipe) < 0) {
+		ret_code = LTTNG_ERR_FATAL;
 		goto error;
 	}
 
-	ret = LTTNG_OK;
+	ret_code = LTTNG_OK;
 error:
 	channel_attr_destroy(defattr);
-	return ret;
+	return ret_code;
 }
 
 /*
  * Enable UST channel for session and domain.
  */
-int channel_ust_enable(struct ltt_ust_session *usess,
+enum lttng_error_code channel_ust_enable(struct ltt_ust_session *usess,
 		struct ltt_ust_channel *uchan)
 {
-	int ret = LTTNG_OK;
+	enum lttng_error_code ret_code = LTTNG_OK;
 
 	LTTNG_ASSERT(usess);
 	LTTNG_ASSERT(uchan);
@@ -295,7 +292,7 @@ int channel_ust_enable(struct ltt_ust_session *usess,
 	/* If already enabled, everything is OK */
 	if (uchan->enabled) {
 		DBG3("Channel %s already enabled. Skipping", uchan->name);
-		ret = LTTNG_ERR_UST_CHAN_EXIST;
+		ret_code = LTTNG_ERR_UST_CHAN_EXIST;
 		goto end;
 	} else {
 		uchan->enabled = 1;
@@ -325,16 +322,16 @@ int channel_ust_enable(struct ltt_ust_session *usess,
 
 
 end:
-	return ret;
+	return ret_code;
 }
 
 /*
  * Create UST channel for session and domain.
  */
-int channel_ust_create(struct ltt_ust_session *usess,
+enum lttng_error_code channel_ust_create(struct ltt_ust_session *usess,
 		struct lttng_channel *attr, enum lttng_buffer_type type)
 {
-	int ret = LTTNG_OK;
+	enum lttng_error_code ret_code = LTTNG_OK;
 	struct ltt_ust_channel *uchan = NULL;
 	struct lttng_channel *defattr = NULL;
 	enum lttng_domain_type domain = LTTNG_DOMAIN_UST;
@@ -346,7 +343,7 @@ int channel_ust_create(struct ltt_ust_session *usess,
 	if (attr == NULL) {
 		defattr = channel_new_default_attr(LTTNG_DOMAIN_UST, type);
 		if (defattr == NULL) {
-			ret = LTTNG_ERR_FATAL;
+			ret_code = LTTNG_ERR_FATAL;
 			goto error;
 		}
 		attr = defattr;
@@ -379,7 +376,7 @@ int channel_ust_create(struct ltt_ust_session *usess,
 
 	/* Validate common channel properties. */
 	if (channel_validate(attr) < 0) {
-		ret = LTTNG_ERR_INVALID;
+		ret_code = LTTNG_ERR_INVALID;
 		goto error;
 	}
 
@@ -390,7 +387,7 @@ int channel_ust_create(struct ltt_ust_session *usess,
 	 */
 	if (!attr->attr.subbuf_size ||
 			(attr->attr.subbuf_size & (attr->attr.subbuf_size - 1))) {
-		ret = LTTNG_ERR_INVALID;
+		ret_code = LTTNG_ERR_INVALID;
 		goto error;
 	}
 
@@ -398,18 +395,18 @@ int channel_ust_create(struct ltt_ust_session *usess,
 	 * Invalid subbuffer size if it's lower then the page size.
 	 */
 	if (attr->attr.subbuf_size < the_page_size) {
-		ret = LTTNG_ERR_INVALID;
+		ret_code = LTTNG_ERR_INVALID;
 		goto error;
 	}
 
 	if (!attr->attr.num_subbuf ||
 			(attr->attr.num_subbuf & (attr->attr.num_subbuf - 1))) {
-		ret = LTTNG_ERR_INVALID;
+		ret_code = LTTNG_ERR_INVALID;
 		goto error;
 	}
 
 	if (attr->attr.output != LTTNG_EVENT_MMAP) {
-		ret = LTTNG_ERR_NOT_SUPPORTED;
+		ret_code = LTTNG_ERR_NOT_SUPPORTED;
 		goto error;
 	}
 
@@ -419,7 +416,7 @@ int channel_ust_create(struct ltt_ust_session *usess,
 	 */
 	if ((attr->attr.tracefile_size > 0) &&
 			(attr->attr.tracefile_size < attr->attr.subbuf_size)) {
-		ret = LTTNG_ERR_INVALID;
+		ret_code = LTTNG_ERR_INVALID;
 		goto error;
 	}
 
@@ -430,20 +427,20 @@ int channel_ust_create(struct ltt_ust_session *usess,
 	case LTTNG_BUFFER_PER_UID:
 		break;
 	default:
-		ret = LTTNG_ERR_BUFFER_NOT_SUPPORTED;
+		ret_code = LTTNG_ERR_BUFFER_NOT_SUPPORTED;
 		goto error;
 	}
 
 	/* Create UST channel */
 	uchan = trace_ust_create_channel(attr, domain);
 	if (uchan == NULL) {
-		ret = LTTNG_ERR_FATAL;
+		ret_code = LTTNG_ERR_FATAL;
 		goto error;
 	}
 
 	uchan->enabled = 1;
 	if (trace_ust_is_max_id(usess->used_channel_id)) {
-		ret = LTTNG_ERR_UST_CHAN_FAIL;
+		ret_code = LTTNG_ERR_UST_CHAN_FAIL;
 		goto error;
 	}
 	uchan->id = trace_ust_get_next_chan_id(usess);
@@ -457,7 +454,7 @@ int channel_ust_create(struct ltt_ust_session *usess,
 		usess->buffer_type_changed = 1;
 	} else if (usess->buffer_type != type) {
 		/* Buffer type was already set. Refuse to create channel. */
-		ret = LTTNG_ERR_BUFFER_TYPE_MISMATCH;
+		ret_code = LTTNG_ERR_BUFFER_TYPE_MISMATCH;
 		goto error_free_chan;
 	}
 
@@ -485,7 +482,7 @@ int channel_ust_create(struct ltt_ust_session *usess,
 		if (!agt) {
 			agt = agent_create(domain);
 			if (!agt) {
-				ret = LTTNG_ERR_NOMEM;
+				ret_code = LTTNG_ERR_NOMEM;
 				goto error_remove_chan;
 			}
 			agent_add(agt, usess->agents);
@@ -503,7 +500,7 @@ error_free_chan:
 	trace_ust_destroy_channel(uchan);
 error:
 	channel_attr_destroy(defattr);
-	return ret;
+	return ret_code;
 }
 
 /*
diff --git a/src/bin/lttng-sessiond/channel.h b/src/bin/lttng-sessiond/channel.h
index 1b6e29bd7..fcc4d6b63 100644
--- a/src/bin/lttng-sessiond/channel.h
+++ b/src/bin/lttng-sessiond/channel.h
@@ -15,18 +15,19 @@
 
 int channel_kernel_disable(struct ltt_kernel_session *ksession,
 		char *channel_name);
-int channel_kernel_enable(struct ltt_kernel_session *ksession,
+enum lttng_error_code channel_kernel_enable(struct ltt_kernel_session *ksession,
 		struct ltt_kernel_channel *kchan);
-int channel_kernel_create(struct ltt_kernel_session *ksession,
-		struct lttng_channel *chan, int kernel_pipe);
+enum lttng_error_code channel_kernel_create(struct ltt_kernel_session *ksession,
+		struct lttng_channel *chan,
+		int kernel_pipe);
 
 struct lttng_channel *channel_new_default_attr(int domain,
 		enum lttng_buffer_type type);
 void channel_attr_destroy(struct lttng_channel *channel);
 
-int channel_ust_create(struct ltt_ust_session *usess,
+enum lttng_error_code channel_ust_create(struct ltt_ust_session *usess,
 		struct lttng_channel *attr, enum lttng_buffer_type type);
-int channel_ust_enable(struct ltt_ust_session *usess,
+enum lttng_error_code channel_ust_enable(struct ltt_ust_session *usess,
 		struct ltt_ust_channel *uchan);
 int channel_ust_disable(struct ltt_ust_session *usess,
 		struct ltt_ust_channel *uchan);
diff --git a/src/bin/lttng-sessiond/cmd.cpp b/src/bin/lttng-sessiond/cmd.cpp
index 4892e3539..3ed56f7b0 100644
--- a/src/bin/lttng-sessiond/cmd.cpp
+++ b/src/bin/lttng-sessiond/cmd.cpp
@@ -114,7 +114,8 @@ static int cmd_enable_event_internal(struct ltt_session *session,
 		struct lttng_bytecode *filter,
 		struct lttng_event_exclusion *exclusion,
 		int wpipe);
-static int cmd_enable_channel_internal(struct ltt_session *session,
+static enum lttng_error_code cmd_enable_channel_internal(
+		struct ltt_session *session,
 		const struct lttng_domain *domain,
 		const struct lttng_channel *_attr,
 		int wpipe);
@@ -1273,12 +1274,13 @@ end:
 	return ret;
 }
 
-static int cmd_enable_channel_internal(struct ltt_session *session,
+static enum lttng_error_code cmd_enable_channel_internal(
+		struct ltt_session *session,
 		const struct lttng_domain *domain,
 		const struct lttng_channel *_attr,
 		int wpipe)
 {
-	int ret;
+	enum lttng_error_code ret_code;
 	struct ltt_ust_session *usess = session->ust_session;
 	struct lttng_ht *chan_ht;
 	size_t len;
@@ -1290,7 +1292,7 @@ static int cmd_enable_channel_internal(struct ltt_session *session,
 
 	attr = lttng_channel_copy(_attr);
 	if (!attr) {
-		ret = LTTNG_ERR_NOMEM;
+		ret_code = LTTNG_ERR_NOMEM;
 		goto end;
 	}
 
@@ -1299,7 +1301,7 @@ static int cmd_enable_channel_internal(struct ltt_session *session,
 	/* Validate channel name */
 	if (attr->name[0] == '.' ||
 		memchr(attr->name, '/', len) != NULL) {
-		ret = LTTNG_ERR_INVALID_CHANNEL_NAME;
+		ret_code = LTTNG_ERR_INVALID_CHANNEL_NAME;
 		goto end;
 	}
 
@@ -1338,12 +1340,12 @@ static int cmd_enable_channel_internal(struct ltt_session *session,
 	case LTTNG_DOMAIN_PYTHON:
 		if (!agent_tracing_is_enabled()) {
 			DBG("Attempted to enable a channel in an agent domain but the agent thread is not running");
-			ret = LTTNG_ERR_AGENT_TRACING_DISABLED;
+			ret_code = LTTNG_ERR_AGENT_TRACING_DISABLED;
 			goto error;
 		}
 		break;
 	default:
-		ret = LTTNG_ERR_UNKNOWN_DOMAIN;
+		ret_code = LTTNG_ERR_UNKNOWN_DOMAIN;
 		goto error;
 	}
 
@@ -1360,7 +1362,7 @@ static int cmd_enable_channel_internal(struct ltt_session *session,
 			 * some point in time before. The tracer does not allow it.
 			 */
 			if (session->has_been_started) {
-				ret = LTTNG_ERR_TRACE_ALREADY_STARTED;
+				ret_code = LTTNG_ERR_TRACE_ALREADY_STARTED;
 				goto error;
 			}
 
@@ -1369,16 +1371,16 @@ static int cmd_enable_channel_internal(struct ltt_session *session,
 				/* Enforce mmap output for snapshot sessions. */
 				attr->attr.output = LTTNG_EVENT_MMAP;
 			}
-			ret = channel_kernel_create(
+			ret_code = channel_kernel_create(
 					session->kernel_session, attr, wpipe);
 			if (attr->name[0] != '\0') {
 				session->kernel_session->has_non_default_channel = 1;
 			}
 		} else {
-			ret = channel_kernel_enable(session->kernel_session, kchan);
+			ret_code = channel_kernel_enable(session->kernel_session, kchan);
 		}
 
-		if (ret != LTTNG_OK) {
+		if (ret_code != LTTNG_OK) {
 			goto error;
 		}
 
@@ -1403,19 +1405,19 @@ static int cmd_enable_channel_internal(struct ltt_session *session,
 		if (domain->type == LTTNG_DOMAIN_JUL) {
 			if (strncmp(attr->name, DEFAULT_JUL_CHANNEL_NAME,
 					LTTNG_SYMBOL_NAME_LEN)) {
-				ret = LTTNG_ERR_INVALID_CHANNEL_NAME;
+				ret_code = LTTNG_ERR_INVALID_CHANNEL_NAME;
 				goto error;
 			}
 		} else if (domain->type == LTTNG_DOMAIN_LOG4J) {
 			if (strncmp(attr->name, DEFAULT_LOG4J_CHANNEL_NAME,
 					LTTNG_SYMBOL_NAME_LEN)) {
-				ret = LTTNG_ERR_INVALID_CHANNEL_NAME;
+				ret_code = LTTNG_ERR_INVALID_CHANNEL_NAME;
 				goto error;
 			}
 		} else if (domain->type == LTTNG_DOMAIN_PYTHON) {
 			if (strncmp(attr->name, DEFAULT_PYTHON_CHANNEL_NAME,
 					LTTNG_SYMBOL_NAME_LEN)) {
-				ret = LTTNG_ERR_INVALID_CHANNEL_NAME;
+				ret_code = LTTNG_ERR_INVALID_CHANNEL_NAME;
 				goto error;
 			}
 		}
@@ -1429,32 +1431,32 @@ static int cmd_enable_channel_internal(struct ltt_session *session,
 			 * some point in time before. The tracer does not allow it.
 			 */
 			if (session->has_been_started) {
-				ret = LTTNG_ERR_TRACE_ALREADY_STARTED;
+				ret_code = LTTNG_ERR_TRACE_ALREADY_STARTED;
 				goto error;
 			}
 
-			ret = channel_ust_create(usess, attr, domain->buf_type);
+			ret_code = channel_ust_create(usess, attr, domain->buf_type);
 			if (attr->name[0] != '\0') {
 				usess->has_non_default_channel = 1;
 			}
 		} else {
-			ret = channel_ust_enable(usess, uchan);
+			ret_code = channel_ust_enable(usess, uchan);
 		}
 		break;
 	}
 	default:
-		ret = LTTNG_ERR_UNKNOWN_DOMAIN;
+		ret_code = LTTNG_ERR_UNKNOWN_DOMAIN;
 		goto error;
 	}
 
-	if (ret == LTTNG_OK && attr->attr.output != LTTNG_EVENT_MMAP) {
+	if (ret_code == LTTNG_OK && attr->attr.output != LTTNG_EVENT_MMAP) {
 		session->has_non_mmap_channel = true;
 	}
 error:
 	rcu_read_unlock();
 end:
 	lttng_channel_destroy(attr);
-	return ret;
+	return ret_code;
 }
 
 enum lttng_error_code cmd_process_attr_tracker_get_tracking_policy(