From: Francis Deslauriers <francis.deslauriers@efficios.com>
Date: Thu, 12 Dec 2019 14:32:00 +0000 (-0500)
Subject: liblttng-ust-comm: move `_unlock_fd_tracker()` after `close()` on error paths
X-Git-Tag: v2.13.0-rc1~536
X-Git-Url: https://git.lttng.org./?a=commitdiff_plain;h=20d1999d42392063fd530d1f6c00b93c25d178d6;p=lttng-ust.git

liblttng-ust-comm: move `_unlock_fd_tracker()` after `close()` on error paths

Right now, receiving an error from `lttng_ust_add_fd_to_tracker()` means that
the fd was _not_ added to the fd set. So the `lttng_ust_safe_close_fd()`
(overriding `close()`) call will indeed close the fd as expected. So, it
doesn't matter if the `close()` is before or after the `_unlock_`.

Even considering that, I believe that it's clearer and more common to
have all related operations within the `_lock_` and  `_unlock_`
functions. Also, `lttng_ust_add_fd_to_tracker()` might be modified in
the future and fail for some other reason.

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Change-Id: Id29a6ab004cfd5ca601615e1a70c74cf754b12e2
---

diff --git a/liblttng-ust-comm/lttng-ust-comm.c b/liblttng-ust-comm/lttng-ust-comm.c
index 543465ed..9f2bdd5b 100644
--- a/liblttng-ust-comm/lttng-ust-comm.c
+++ b/liblttng-ust-comm/lttng-ust-comm.c
@@ -631,12 +631,12 @@ ssize_t ustcomm_recv_channel_from_sessiond(int sock,
 
 	ret = lttng_ust_add_fd_to_tracker(wakeup_fd);
 	if (ret < 0) {
-		lttng_ust_unlock_fd_tracker();
 		ret = close(wakeup_fd);
 		if (ret) {
 			PERROR("close on wakeup_fd");
 		}
 		len = -EIO;
+		lttng_ust_unlock_fd_tracker();
 		goto error_recv;
 	}
 
@@ -677,19 +677,18 @@ int ustcomm_recv_stream_from_sessiond(int sock,
 
 	ret = lttng_ust_add_fd_to_tracker(fds[0]);
 	if (ret < 0) {
-		lttng_ust_unlock_fd_tracker();
 		ret = close(fds[0]);
 		if (ret) {
 			PERROR("close on received shm_fd");
 		}
 		ret = -EIO;
+		lttng_ust_unlock_fd_tracker();
 		goto error;
 	}
 	*shm_fd = ret;
 
 	ret = lttng_ust_add_fd_to_tracker(fds[1]);
 	if (ret < 0) {
-		lttng_ust_unlock_fd_tracker();
 		ret = close(*shm_fd);
 		if (ret) {
 			PERROR("close on shm_fd");
@@ -700,6 +699,7 @@ int ustcomm_recv_stream_from_sessiond(int sock,
 			PERROR("close on received wakeup_fd");
 		}
 		ret = -EIO;
+		lttng_ust_unlock_fd_tracker();
 		goto error;
 	}
 	*wakeup_fd = ret;