[PATCH iproute2 0/3] some memory leak fixes

[PATCH iproute2 0/3] some memory leak fixes

[Andrea Claudi]

This series fixes some memory leaks related to the usage of the
get_task_name() function from lib/fs.c.

Patch 3/3 addresses a coverity warning related to this memory leak,
making the code a bit more readable by humans and coverity.

Andrea Claudi (3):
  lib/fs: fix memory leak in get_task_name()
  rdma: stat: fix memory leak in res_counter_line()
  rdma: RES_PID and RES_KERN_NAME are alternatives to each other

 lib/fs.c        | 10 +++++--
 rdma/res-cmid.c | 10 +++----
 rdma/res-cq.c   |  9 +++---
 rdma/res-ctx.c  |  9 +++---
 rdma/res-mr.c   |  8 ++---
 rdma/res-pd.c   |  9 +++---
 rdma/res-qp.c   |  9 +++---
 rdma/res-srq.c  | 10 +++----
 rdma/stat.c     | 79 ++++++++++++++++++++++++++++++++-----------------
 9 files changed, 88 insertions(+), 65 deletions(-)

-- 
2.34.1

[PATCH iproute2 1/3] lib/fs: fix memory leak in get_task_name()

[Andrea Claudi]

asprintf() allocates memory which is not freed on the error path of
get_task_name(), thus potentially leading to memory leaks.

This commit fixes this using free() on error paths.

Fixes: 81bfd01a4c9e ("lib: move get_task_name() from rdma")
Signed-off-by: Andrea Claudi <aclaudi@redhat.com>
---
 lib/fs.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/lib/fs.c b/lib/fs.c
index f6f5f8a0..5692e2d3 100644
--- a/lib/fs.c
+++ b/lib/fs.c
@@ -354,11 +354,15 @@ char *get_task_name(pid_t pid)
 		return NULL;
 
 	f = fopen(comm, "r");
-	if (!f)
+	if (!f) {
+		free(comm);
 		return NULL;
+	}
 
-	if (fscanf(f, "%ms\n", &comm) != 1)
-		comm = NULL;
+	if (fscanf(f, "%ms\n", &comm) != 1) {
+		free(comm);
+		return NULL;
+	}
 
 	fclose(f);
 
-- 
2.34.1

[PATCH iproute2 2/3] rdma: stat: fix memory leak in res_counter_line()

[Andrea Claudi]

get_task_name() uses asprintf() to reserve storage for the task name
string. As asprintf() manpage states, a free() should be used to release
the allocated storage when it is no longer needed.

res_counter_line() uses get_task_name() but does not free the allocated
storage on the error paths and after the usage.

This uses a single return point to free the allocated storage, adopting
the same approach of other rdma code using get_task_name().

Fixes: 5937552b42e4 ("rdma: Add "stat qp show" support")
Signed-off-by: Andrea Claudi <aclaudi@redhat.com>
---
 rdma/stat.c | 74 ++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 50 insertions(+), 24 deletions(-)

diff --git a/rdma/stat.c b/rdma/stat.c
index adfcd34a..66121b12 100644
--- a/rdma/stat.c
+++ b/rdma/stat.c
@@ -222,9 +222,9 @@ static int res_counter_line(struct rd *rd, const char *name, int index,
 	uint32_t cntn, port = 0, pid = 0, qpn, qp_type = 0;
 	struct nlattr *hwc_table, *qp_table;
 	struct nlattr *nla_entry;
-	const char *comm = NULL;
+	char *comm = NULL;
 	bool isfirst;
-	int err;
+	int err, ret;
 
 	if (nla_line[RDMA_NLDEV_ATTR_PORT_INDEX])
 		port = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_PORT_INDEX]);
@@ -232,52 +232,70 @@ static int res_counter_line(struct rd *rd, const char *name, int index,
 	hwc_table = nla_line[RDMA_NLDEV_ATTR_STAT_HWCOUNTERS];
 	qp_table = nla_line[RDMA_NLDEV_ATTR_RES_QP];
 	if (!hwc_table || !qp_table ||
-	    !nla_line[RDMA_NLDEV_ATTR_STAT_COUNTER_ID])
-		return MNL_CB_ERROR;
+	    !nla_line[RDMA_NLDEV_ATTR_STAT_COUNTER_ID]) {
+		ret = MNL_CB_ERROR;
+		goto out;
+	}
 
 	cntn = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_STAT_COUNTER_ID]);
 	if (rd_is_filtered_attr(rd, "cntn", cntn,
-				nla_line[RDMA_NLDEV_ATTR_STAT_COUNTER_ID]))
-		return MNL_CB_OK;
+				nla_line[RDMA_NLDEV_ATTR_STAT_COUNTER_ID])) {
+		ret = MNL_CB_OK;
+		goto out;
+	}
 
 	if (nla_line[RDMA_NLDEV_ATTR_RES_TYPE])
 		qp_type = mnl_attr_get_u8(nla_line[RDMA_NLDEV_ATTR_RES_TYPE]);
 
 	if (rd_is_string_filtered_attr(rd, "qp-type", qp_types_to_str(qp_type),
-				       nla_line[RDMA_NLDEV_ATTR_RES_TYPE]))
-		return MNL_CB_OK;
+				       nla_line[RDMA_NLDEV_ATTR_RES_TYPE])) {
+		ret = MNL_CB_OK;
+		goto out;
+	}
 
 	if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) {
 		pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]);
 		comm = get_task_name(pid);
 	}
 	if (rd_is_filtered_attr(rd, "pid", pid,
-				nla_line[RDMA_NLDEV_ATTR_RES_PID]))
-		return MNL_CB_OK;
+				nla_line[RDMA_NLDEV_ATTR_RES_PID])) {
+		ret = MNL_CB_OK;
+		goto out;
+	}
 
-	if (nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME])
+	if (nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]) {
+		/* discard const from mnl_attr_get_str */
 		comm = (char *)mnl_attr_get_str(
 			nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]);
+	}
 
 	mnl_attr_for_each_nested(nla_entry, qp_table) {
 		struct nlattr *qp_line[RDMA_NLDEV_ATTR_MAX] = {};
 
 		err = mnl_attr_parse_nested(nla_entry, rd_attr_cb, qp_line);
-		if (err != MNL_CB_OK)
-			return -EINVAL;
+		if (err != MNL_CB_OK) {
+			ret = -EINVAL;
+			goto out;
+		}
 
-		if (!qp_line[RDMA_NLDEV_ATTR_RES_LQPN])
-			return -EINVAL;
+		if (!qp_line[RDMA_NLDEV_ATTR_RES_LQPN]) {
+			ret = -EINVAL;
+			goto out;
+		}
 
 		qpn = mnl_attr_get_u32(qp_line[RDMA_NLDEV_ATTR_RES_LQPN]);
 		if (rd_is_filtered_attr(rd, "lqpn", qpn,
-					qp_line[RDMA_NLDEV_ATTR_RES_LQPN]))
-			return MNL_CB_OK;
+					qp_line[RDMA_NLDEV_ATTR_RES_LQPN])) {
+			ret = MNL_CB_OK;
+			goto out;
+		}
 	}
 
 	err = res_get_hwcounters(rd, hwc_table, false);
-	if (err != MNL_CB_OK)
-		return err;
+	if (err != MNL_CB_OK) {
+		ret = err;
+		goto out;
+	}
 	open_json_object(NULL);
 	print_link(rd, index, name, port, nla_line);
 	print_color_uint(PRINT_ANY, COLOR_NONE, "cntn", "cntn %u ", cntn);
@@ -292,11 +310,15 @@ static int res_counter_line(struct rd *rd, const char *name, int index,
 	mnl_attr_for_each_nested(nla_entry, qp_table) {
 		struct nlattr *qp_line[RDMA_NLDEV_ATTR_MAX] = {};
 		err = mnl_attr_parse_nested(nla_entry, rd_attr_cb, qp_line);
-		if (err != MNL_CB_OK)
-			return -EINVAL;
+		if (err != MNL_CB_OK) {
+			ret = -EINVAL;
+			goto out;
+		}
 
-		if (!qp_line[RDMA_NLDEV_ATTR_RES_LQPN])
-			return -EINVAL;
+		if (!qp_line[RDMA_NLDEV_ATTR_RES_LQPN]) {
+			ret = -EINVAL;
+			goto out;
+		}
 
 		qpn = mnl_attr_get_u32(qp_line[RDMA_NLDEV_ATTR_RES_LQPN]);
 		if (!isfirst)
@@ -307,7 +329,11 @@ static int res_counter_line(struct rd *rd, const char *name, int index,
 	}
 	close_json_array(PRINT_ANY, ">");
 	newline(rd);
-	return MNL_CB_OK;
+	ret = MNL_CB_OK;
+out:
+	if (nla_line[RDMA_NLDEV_ATTR_RES_PID])
+		free(comm);
+	return ret;
 }
 
 static int stat_qp_show_parse_cb(const struct nlmsghdr *nlh, void *data)
-- 
2.34.1

[PATCH iproute2 3/3] rdma: RES_PID and RES_KERN_NAME are alternatives to each other

[Andrea Claudi]

RDMA_NLDEV_ATTR_RES_PID and RDMA_NLDEV_ATTR_RES_KERN_NAME cannot be set
together, as evident from the fill_res_name_pid() function in the kernel
infiniband driver. This commit makes this clear at first glance, using
an else branch for RDMA_NLDEV_ATTR_RES_KERN_NAME.

This also helps coverity to better understand this code and avoid
producing a bogus warning complaining about mnl_attr_get_str overwriting
comm, and thus leaking the storage that comm points to.

Signed-off-by: Andrea Claudi <aclaudi@redhat.com>
---
 rdma/res-cmid.c | 10 ++++------
 rdma/res-cq.c   |  9 ++++-----
 rdma/res-ctx.c  |  9 ++++-----
 rdma/res-mr.c   |  8 ++++----
 rdma/res-pd.c   |  9 ++++-----
 rdma/res-qp.c   |  9 ++++-----
 rdma/res-srq.c  | 10 +++++-----
 rdma/stat.c     | 11 +++++------
 8 files changed, 34 insertions(+), 41 deletions(-)

diff --git a/rdma/res-cmid.c b/rdma/res-cmid.c
index bfaa47b5..21bdabca 100644
--- a/rdma/res-cmid.c
+++ b/rdma/res-cmid.c
@@ -161,6 +161,10 @@ static int res_cm_id_line(struct rd *rd, const char *name, int idx,
 	if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) {
 		pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]);
 		comm = get_task_name(pid);
+	} else if (nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]) {
+		/* discard const from mnl_attr_get_str */
+		comm = (char *)mnl_attr_get_str(
+			nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]);
 	}
 
 	if (rd_is_filtered_attr(rd, "pid", pid,
@@ -173,12 +177,6 @@ static int res_cm_id_line(struct rd *rd, const char *name, int idx,
 				nla_line[RDMA_NLDEV_ATTR_RES_CM_IDN]))
 		goto out;
 
-	if (nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]) {
-		/* discard const from mnl_attr_get_str */
-		comm = (char *)mnl_attr_get_str(
-			nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]);
-	}
-
 	open_json_object(NULL);
 	print_link(rd, idx, name, port, nla_line);
 	res_print_uint(rd, "cm-idn", cm_idn,
diff --git a/rdma/res-cq.c b/rdma/res-cq.c
index 9e7c4f51..0ba19447 100644
--- a/rdma/res-cq.c
+++ b/rdma/res-cq.c
@@ -86,6 +86,10 @@ static int res_cq_line(struct rd *rd, const char *name, int idx,
 	if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) {
 		pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]);
 		comm = get_task_name(pid);
+	} else if (nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]) {
+		/* discard const from mnl_attr_get_str */
+		comm = (char *)mnl_attr_get_str(
+			nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]);
 	}
 
 	if (rd_is_filtered_attr(rd, "pid", pid,
@@ -103,11 +107,6 @@ static int res_cq_line(struct rd *rd, const char *name, int idx,
 				nla_line[RDMA_NLDEV_ATTR_RES_CTXN]))
 		goto out;
 
-	if (nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME])
-		/* discard const from mnl_attr_get_str */
-		comm = (char *)mnl_attr_get_str(
-			nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]);
-
 	open_json_object(NULL);
 	print_dev(rd, idx, name);
 	res_print_uint(rd, "cqn", cqn, nla_line[RDMA_NLDEV_ATTR_RES_CQN]);
diff --git a/rdma/res-ctx.c b/rdma/res-ctx.c
index 30afe97a..895598d8 100644
--- a/rdma/res-ctx.c
+++ b/rdma/res-ctx.c
@@ -20,6 +20,10 @@ static int res_ctx_line(struct rd *rd, const char *name, int idx,
 	if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) {
 		pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]);
 		comm = get_task_name(pid);
+	} else if (nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]) {
+		/* discard const from mnl_attr_get_str */
+		comm = (char *)mnl_attr_get_str(
+			nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]);
 	}
 
 	if (rd_is_filtered_attr(rd, "pid", pid,
@@ -33,11 +37,6 @@ static int res_ctx_line(struct rd *rd, const char *name, int idx,
 				nla_line[RDMA_NLDEV_ATTR_RES_CTXN]))
 		goto out;
 
-	if (nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME])
-		/* discard const from mnl_attr_get_str */
-		comm = (char *)mnl_attr_get_str(
-			nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]);
-
 	open_json_object(NULL);
 	print_dev(rd, idx, name);
 	res_print_uint(rd, "ctxn", ctxn, nla_line[RDMA_NLDEV_ATTR_RES_CTXN]);
diff --git a/rdma/res-mr.c b/rdma/res-mr.c
index 1bf73f3a..9ae6c777 100644
--- a/rdma/res-mr.c
+++ b/rdma/res-mr.c
@@ -49,6 +49,10 @@ static int res_mr_line(struct rd *rd, const char *name, int idx,
 	if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) {
 		pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]);
 		comm = get_task_name(pid);
+	} else if (nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]) {
+		/* discard const from mnl_attr_get_str */
+		comm = (char *)mnl_attr_get_str(
+			nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]);
 	}
 
 	if (rd_is_filtered_attr(rd, "pid", pid,
@@ -67,10 +71,6 @@ static int res_mr_line(struct rd *rd, const char *name, int idx,
 				nla_line[RDMA_NLDEV_ATTR_RES_PDN]))
 		goto out;
 
-	if (nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME])
-		/* discard const from mnl_attr_get_str */
-		comm = (char *)mnl_attr_get_str(
-			nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]);
 	open_json_object(NULL);
 	print_dev(rd, idx, name);
 	res_print_uint(rd, "mrn", mrn, nla_line[RDMA_NLDEV_ATTR_RES_MRN]);
diff --git a/rdma/res-pd.c b/rdma/res-pd.c
index df538010..aca2b0cc 100644
--- a/rdma/res-pd.c
+++ b/rdma/res-pd.c
@@ -36,6 +36,10 @@ static int res_pd_line(struct rd *rd, const char *name, int idx,
 	if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) {
 		pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]);
 		comm = get_task_name(pid);
+	} else if (nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]) {
+		/* discard const from mnl_attr_get_str */
+		comm = (char *)mnl_attr_get_str(
+			nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]);
 	}
 
 	if (rd_is_filtered_attr(rd, "pid", pid,
@@ -55,11 +59,6 @@ static int res_pd_line(struct rd *rd, const char *name, int idx,
 				nla_line[RDMA_NLDEV_ATTR_RES_PDN]))
 		goto out;
 
-	if (nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME])
-		/* discard const from mnl_attr_get_str */
-		comm = (char *)mnl_attr_get_str(
-			nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]);
-
 	open_json_object(NULL);
 	print_dev(rd, idx, name);
 	res_print_uint(rd, "pdn", pdn, nla_line[RDMA_NLDEV_ATTR_RES_PDN]);
diff --git a/rdma/res-qp.c b/rdma/res-qp.c
index a38be399..76a5334f 100644
--- a/rdma/res-qp.c
+++ b/rdma/res-qp.c
@@ -148,17 +148,16 @@ static int res_qp_line(struct rd *rd, const char *name, int idx,
 	if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) {
 		pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]);
 		comm = get_task_name(pid);
+	} else if (nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]) {
+		/* discard const from mnl_attr_get_str */
+		comm = (char *)mnl_attr_get_str(
+			nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]);
 	}
 
 	if (rd_is_filtered_attr(rd, "pid", pid,
 				nla_line[RDMA_NLDEV_ATTR_RES_PID]))
 		goto out;
 
-	if (nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME])
-		/* discard const from mnl_attr_get_str */
-		comm = (char *)mnl_attr_get_str(
-			nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]);
-
 	open_json_object(NULL);
 	print_link(rd, idx, name, port, nla_line);
 	res_print_uint(rd, "lqpn", lqpn, nla_line[RDMA_NLDEV_ATTR_RES_LQPN]);
diff --git a/rdma/res-srq.c b/rdma/res-srq.c
index 3038c352..982e3fe9 100644
--- a/rdma/res-srq.c
+++ b/rdma/res-srq.c
@@ -176,7 +176,12 @@ static int res_srq_line(struct rd *rd, const char *name, int idx,
 	if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) {
 		pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]);
 		comm = get_task_name(pid);
+	} else if (nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]) {
+		/* discard const from mnl_attr_get_str */
+		comm = (char *)mnl_attr_get_str(
+			nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]);
 	}
+
 	if (rd_is_filtered_attr(rd, "pid", pid,
 				nla_line[RDMA_NLDEV_ATTR_RES_PID]))
 		goto out;
@@ -209,11 +214,6 @@ static int res_srq_line(struct rd *rd, const char *name, int idx,
 			MNL_CB_OK)
 		goto out;
 
-	if (nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME])
-		/* discard const from mnl_attr_get_str */
-		comm = (char *)mnl_attr_get_str(
-			nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]);
-
 	open_json_object(NULL);
 	print_dev(rd, idx, name);
 	res_print_uint(rd, "srqn", srqn, nla_line[RDMA_NLDEV_ATTR_RES_SRQN]);
diff --git a/rdma/stat.c b/rdma/stat.c
index 66121b12..cf2c705b 100644
--- a/rdma/stat.c
+++ b/rdma/stat.c
@@ -256,19 +256,18 @@ static int res_counter_line(struct rd *rd, const char *name, int index,
 	if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) {
 		pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]);
 		comm = get_task_name(pid);
+	} else if (nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]) {
+		/* discard const from mnl_attr_get_str */
+		comm = (char *)mnl_attr_get_str(
+			nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]);
 	}
+
 	if (rd_is_filtered_attr(rd, "pid", pid,
 				nla_line[RDMA_NLDEV_ATTR_RES_PID])) {
 		ret = MNL_CB_OK;
 		goto out;
 	}
 
-	if (nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]) {
-		/* discard const from mnl_attr_get_str */
-		comm = (char *)mnl_attr_get_str(
-			nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]);
-	}
-
 	mnl_attr_for_each_nested(nla_entry, qp_table) {
 		struct nlattr *qp_line[RDMA_NLDEV_ATTR_MAX] = {};
 
-- 
2.34.1

Re: [PATCH iproute2 1/3] lib/fs: fix memory leak in get_task_name()

[Stephen Hemminger]

On Tue,  1 Feb 2022 18:39:24 +0100
Andrea Claudi <aclaudi@redhat.com> wrote:

> +	if (fscanf(f, "%ms\n", &comm) != 1) {
> +		free(comm);
> +		return NULL;

This is still leaking the original comm.

Why not change it to use a local variable for the path
(avoid asprintf) and not reuse comm for both pathname
and return value.

Re: [PATCH iproute2 1/3] lib/fs: fix memory leak in get_task_name()

[Andrea Claudi]

On Tue, Feb 01, 2022 at 10:27:12AM -0800, Stephen Hemminger wrote:
> On Tue,  1 Feb 2022 18:39:24 +0100
> Andrea Claudi <aclaudi@redhat.com> wrote:
> 
> > +	if (fscanf(f, "%ms\n", &comm) != 1) {
> > +		free(comm);
> > +		return NULL;
> 
> This is still leaking the original comm.
>

Thanks Stephen, I missed the %m over there :(

> Why not change it to use a local variable for the path
> (avoid asprintf) and not reuse comm for both pathname
> and return value.
> 

Thanks for the suggestion. 

What about taking an extra-step and get rid of the %m too?
We can do something similar to the get_command_name() function so that
we don't need to use free in places where we use get_task_name().

Re: [PATCH iproute2 1/3] lib/fs: fix memory leak in get_task_name()

[Stephen Hemminger]

On Wed, 2 Feb 2022 15:38:16 +0100
Andrea Claudi <aclaudi@redhat.com> wrote:

> On Tue, Feb 01, 2022 at 10:27:12AM -0800, Stephen Hemminger wrote:
> > On Tue,  1 Feb 2022 18:39:24 +0100
> > Andrea Claudi <aclaudi@redhat.com> wrote:
> >   
> > > +	if (fscanf(f, "%ms\n", &comm) != 1) {
> > > +		free(comm);
> > > +		return NULL;  
> > 
> > This is still leaking the original comm.
> >  
> 
> Thanks Stephen, I missed the %m over there :(
> 
> > Why not change it to use a local variable for the path
> > (avoid asprintf) and not reuse comm for both pathname
> > and return value.
> >   
> 
> Thanks for the suggestion. 
> 
> What about taking an extra-step and get rid of the %m too?
> We can do something similar to the get_command_name() function so that
> we don't need to use free in places where we use get_task_name().

What ever works best.