diff mbox series

[libgpiod] gpiosim: fix a resource leak

Message ID 20221205154409.1012081-1-brgl@bgdev.pl
State New
Headers show
Series [libgpiod] gpiosim: fix a resource leak | expand

Commit Message

Bartosz Golaszewski Dec. 5, 2022, 3:44 p.m. UTC 
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

If we set the number of lines to x, set line names or hogs for lines
from 0 through x, then set the number of lines to (x - y), we will not
unlink all the line directories as we only iterate up to num_lines in
bank_release().

Allocate a small structure for every line we create a lineX directory
for and iterate over it in bank_release() to know exactly which ones
need freeing.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 tests/gpiosim/gpiosim.c | 92 ++++++++++++++++++++++++++---------------
 1 file changed, 58 insertions(+), 34 deletions(-)
diff mbox series

Patch

diff --git a/tests/gpiosim/gpiosim.c b/tests/gpiosim/gpiosim.c
index 45a0013..a8410f3 100644
--- a/tests/gpiosim/gpiosim.c
+++ b/tests/gpiosim/gpiosim.c
@@ -228,6 +228,12 @@  static void list_del(struct list_head *entry)
 	     !list_entry_is_head(pos, head, member); \
 	     pos = list_next_entry(pos, member))
 
+#define list_for_each_entry_safe(pos, next, head, member) \
+	for (pos = list_first_entry(head, typeof(*pos), member), \
+	     next = list_next_entry(pos, member); \
+	     !list_entry_is_head(pos, head, member); \
+	     pos = next, next = list_next_entry(next, member))
+
 static int open_write_close(int base_fd, const char *where, const char *what)
 {
 	ssize_t written, size;
@@ -411,6 +417,12 @@  struct gpiosim_bank {
 	int cfs_dir_fd;
 	int sysfs_dir_fd;
 	size_t num_lines;
+	struct list_head lines;
+};
+
+struct gpiosim_line {
+	struct list_head siblings;
+	unsigned int offset;
 };
 
 static inline struct gpiosim_ctx *to_gpiosim_ctx(struct refcount *ref)
@@ -841,15 +853,18 @@  static void bank_release(struct refcount *ref)
 {
 	struct gpiosim_bank *bank = to_gpiosim_bank(ref);
 	struct gpiosim_dev *dev = bank->dev;
-	unsigned int i;
+	struct gpiosim_line *line, *tmp;
 	char buf[64];
 
-	/* FIXME should be based on dirent because num_lines can change. */
-	for (i = 0; i < bank->num_lines; i++) {
-		snprintf(buf, sizeof(buf), "line%u/hog", i);
+	list_for_each_entry_safe(line, tmp, &bank->lines, siblings) {
+		snprintf(buf, sizeof(buf), "line%u/hog", line->offset);
 		unlinkat(bank->cfs_dir_fd, buf, AT_REMOVEDIR);
-		snprintf(buf, sizeof(buf), "line%u", i);
+
+		snprintf(buf, sizeof(buf), "line%u", line->offset);
 		unlinkat(bank->cfs_dir_fd, buf, AT_REMOVEDIR);
+
+		list_del(&line->siblings);
+		free(line);
 	}
 
 	list_del(&bank->siblings);
@@ -900,6 +915,7 @@  GPIOSIM_API struct gpiosim_bank *gpiosim_bank_new(struct gpiosim_dev *dev)
 	bank->item_name = item_name;
 	bank->num_lines = 1;
 	bank->id = id;
+	list_init(&bank->lines);
 
 	return bank;
 
@@ -969,25 +985,34 @@  GPIOSIM_API int gpiosim_bank_set_num_lines(struct gpiosim_bank *bank,
 	return 0;
 }
 
-/*
- * Create a sub-directory under given bank's configfs directory. Do nothing
- * if the directory exists and is writable. Mode is O_RDONLY.
- */
-static int bank_mkdirat(struct gpiosim_bank *bank, const char *path)
+static int bank_make_line_dir(struct gpiosim_bank *bank, unsigned int offset)
 {
+	struct gpiosim_line *line;
+	char buf[32];
 	int ret;
 
-	ret = faccessat(bank->cfs_dir_fd, path, W_OK, 0);
+	snprintf(buf, sizeof(buf), "line%u", offset);
+
+	ret = faccessat(bank->cfs_dir_fd, buf, W_OK, 0);
+	if (!ret)
+		return 0;
+	if (ret && errno != ENOENT)
+		return -1;
+
+	line = malloc(sizeof(*line));
+	if (!line)
+		return -1;
+
+	ret = mkdirat(bank->cfs_dir_fd, buf, O_RDONLY);
 	if (ret) {
-		if (errno == ENOENT) {
-			ret = mkdirat(bank->cfs_dir_fd, path, O_RDONLY);
-			if (ret)
-				return -1;
-		} else {
-			return -1;
-		}
+		free(line);
+		return -1;
 	}
 
+	memset(line, 0, sizeof(*line));
+	line->offset = offset;
+	list_add(&line->siblings, &bank->lines);
+
 	return 0;
 }
 
@@ -996,24 +1021,18 @@  GPIOSIM_API int gpiosim_bank_set_line_name(struct gpiosim_bank *bank,
 					   const char *name)
 {
 	char buf[32];
-	int ret, fd;
+	int ret;
 
 	if (!dev_check_pending(bank->dev))
 		return -1;
 
-	snprintf(buf, sizeof(buf), "line%u", offset);
-
-	ret = bank_mkdirat(bank, buf);
+	ret = bank_make_line_dir(bank, offset);
 	if (ret)
 		return -1;
 
-	fd = openat(bank->cfs_dir_fd, buf, O_RDONLY);
-	if (fd < 0)
-		return -1;
+	snprintf(buf, sizeof(buf), "line%u/name", offset);
 
-	ret = open_write_close(fd, "name", name ?: "");
-	close(fd);
-	return ret;
+	return open_write_close(bank->cfs_dir_fd, buf, name ?: "");
 }
 
 GPIOSIM_API int gpiosim_bank_hog_line(struct gpiosim_bank *bank,
@@ -1040,17 +1059,22 @@  GPIOSIM_API int gpiosim_bank_hog_line(struct gpiosim_bank *bank,
 	if (!dev_check_pending(bank->dev))
 		return -1;
 
-	snprintf(buf, sizeof(buf), "line%u", offset);
-
-	ret = bank_mkdirat(bank, buf);
+	ret = bank_make_line_dir(bank, offset);
 	if (ret)
 		return -1;
 
 	snprintf(buf, sizeof(buf), "line%u/hog", offset);
 
-	ret = bank_mkdirat(bank, buf);
-	if (ret)
-		return -1;
+	ret = faccessat(bank->cfs_dir_fd, buf, W_OK, 0);
+	if (ret) {
+		if (errno == ENOENT) {
+			ret = mkdirat(bank->cfs_dir_fd, buf, O_RDONLY);
+			if (ret)
+				return -1;
+		} else {
+			return -1;
+		}
+	}
 
 	fd = openat(bank->cfs_dir_fd, buf, O_RDONLY);
 	if (fd < 0)