gpiosim: fix a resource leak

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>

diff --git a/tests/gpiosim/gpiosim.c b/tests/gpiosim/gpiosim.c
index 45a001397e3134df0427c451484805fe38a8aa58..a8410f38f00075f37e18f3542bd32ce0840c0824 100644 (file)
--- 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)