Re: [ptest-runner 4/5] mem: Refactor ptest_list cleanup


Anibal Limon
 

Hi,

I tried to apply without success, looks like the patch is corrupted, can you send it again?, or share a git repo/branch.

Regards,
Anibal


On Mon, 26 Jul 2021 at 11:10, Anibal Limon <anibal.limon@...> wrote:
Indeed this change doesn't has a direct impact  but is fine to me.

Applied, Thanks!

On Wed, 21 Jul 2021 at 04:47, ?ukasz Majewski <lukma@...> wrote:
From: Adrian Freihofer <adrian.freihofer@...>

Try to make memory management more robust by assigning always NULL to
struct ptest_list pointers. It's a refactoring which probably improves
the code but does not fix a concrete bug.

Signed-off-by: Adrian Freihofer <adrian.freihofer@...>
---
 main.c             |  9 +++++----
 ptest_list.c       | 13 ++++---------
 ptest_list.h       |  8 +-------
 tests/ptest_list.c | 13 +++++++------
 tests/utils.c      | 22 +++++++++++-----------
 utils.c            |  6 +++---
 6 files changed, 31 insertions(+), 40 deletions(-)

diff --git a/main.c b/main.c
index e73626c..efa21b2 100644
--- a/main.c
+++ b/main.c
@@ -116,7 +116,8 @@ main(int argc, char *argv[])
        mtrace();
 #endif

-       struct ptest_list *head, *run;
+       struct ptest_list *head = NULL;
+       struct ptest_list *run = NULL;
        __attribute__ ((__cleanup__(cleanup_ptest_opts))) struct ptest_options opts;

        opts.dirs = malloc(sizeof(char **) * 1);
@@ -175,7 +176,7 @@ main(int argc, char *argv[])

        head = NULL;
        for (i = 0; i < opts.dirs_no; i ++) {
-               struct ptest_list *tmp;
+               struct ptest_list *tmp = NULL;

                tmp = get_available_ptests(opts.dirs[i], opts.timeout);
                if (tmp == NULL) {
@@ -211,7 +212,7 @@ main(int argc, char *argv[])

                run = filter_ptests(head, opts.ptests, ptest_num);
                CHECK_ALLOCATION(run, (size_t) ptest_num, 1);
-               ptest_list_free_all(head);
+               ptest_list_free_all(&head);
        }

        for (i = 0; i < ptest_exclude_num; i++)
@@ -219,7 +220,7 @@ main(int argc, char *argv[])

        rc = run_ptests(run, opts, argv[0], stdout, stderr);

-       ptest_list_free_all(run);
+       ptest_list_free_all(&run);

        return rc;
 }
diff --git a/ptest_list.c b/ptest_list.c
index b689670..87b8c71 100644
--- a/ptest_list.c
+++ b/ptest_list.c
@@ -69,24 +69,19 @@ ptest_list_free(struct ptest_list *p)
        free(p);
 }

-int
-ptest_list_free_all(struct ptest_list *head)
+void
+ptest_list_free_all(struct ptest_list **head)
 {
-       int i = 0;
        struct ptest_list *p, *q;

-       VALIDATE_PTR_RINT(head);
-
-       p = head;
+       p = *head;
        while (p != NULL) {
                q = p;
                p = p->next;

                ptest_list_free(q);
-               i++;
        }
-
-       return i;
+       *head = NULL;
 }

 int
diff --git a/ptest_list.h b/ptest_list.h
index e583d9f..949250c 100644
--- a/ptest_list.h
+++ b/ptest_list.h
@@ -36,12 +36,6 @@
                x = NULL; \
        } while (0)

-#define PTEST_LIST_FREE_ALL_CLEAN(x) \
-       do { \
-               ptest_list_free_all(x); \
-               x = NULL; \
-       } while (0)
-
 #define PTEST_LIST_ITERATE_START(head, p) for (p = head->next; p != NULL; p = p->next) {
 #define PTEST_LIST_ITERATE_END }

@@ -58,7 +52,7 @@ struct ptest_list {

 extern struct ptest_list *ptest_list_alloc(void);
 extern void ptest_list_free(struct ptest_list *);
-extern int ptest_list_free_all(struct ptest_list *);
+extern void ptest_list_free_all(struct ptest_list **);

 extern int ptest_list_length(struct ptest_list *);
 extern struct ptest_list *ptest_list_search(struct ptest_list *, char *);
diff --git a/tests/ptest_list.c b/tests/ptest_list.c
index 37d19ae..6bbc13b 100644
--- a/tests/ptest_list.c
+++ b/tests/ptest_list.c
@@ -53,7 +53,7 @@ START_TEST(test_add)
 {
        struct ptest_list *head = ptest_list_alloc();
        ck_assert(ptest_list_add(head, strdup("perl"), NULL, 1) != NULL);
-       ptest_list_free_all(head);
+       ptest_list_free_all(&head);
 }
 END_TEST

@@ -62,14 +62,15 @@ START_TEST(test_free_all)
        struct ptest_list *head = NULL;
        int i;

-       ck_assert(ptest_list_free_all(head) == -1);
+       ptest_list_free_all(&head);
+       ck_assert(head == NULL);
        ck_assert(errno == EINVAL);

        head = ptest_list_alloc();
        for (i = 0; i < ptests_num; i++)
                ptest_list_add(head, strdup(ptest_names[i]), NULL, 1);

-       ptest_list_free_all(head);
+       ptest_list_free_all(&head);
 }
 END_TEST

@@ -87,7 +88,7 @@ START_TEST(test_length)
                ptest_list_add(head, strdup(ptest_names[i]), NULL, 1);

        ck_assert_int_eq(ptest_list_length(head), ptests_num);
-       ptest_list_free_all(head);
+       ptest_list_free_all(&head);
 }
 END_TEST

@@ -109,7 +110,7 @@ START_TEST(test_search)
        for (i = ptests_num - 1; i >= 0; i--)
                ck_assert(ptest_list_search(head, ptest_names[i]) != NULL);

-       ptest_list_free_all(head);
+       ptest_list_free_all(&head);
 }
 END_TEST

@@ -141,7 +142,7 @@ START_TEST(test_remove)
        ck_assert_int_eq(ptest_list_length(head), n);

        ck_assert(ptest_list_search(head, "busybox") != NULL);
-       ptest_list_free_all(head);
+       ptest_list_free_all(&head);
 }
 END_TEST

diff --git a/tests/utils.c b/tests/utils.c
index 8df1b54..4e244fe 100644
--- a/tests/utils.c
+++ b/tests/utils.c
@@ -88,13 +88,13 @@ START_TEST(test_get_available_ptests)
        for (i = 0; ptests_not_found[i] != NULL; i++)
                ck_assert(ptest_list_search(head, ptests_not_found[i]) == NULL);

-       ptest_list_free_all(head);
+       ptest_list_free_all(&head);
 }
 END_TEST

 START_TEST(test_print_ptests)
 {
-       struct ptest_list *head;
+       struct ptest_list *head = NULL;

        char *buf;
        size_t size = PRINT_PTEST_BUF_SIZE;
@@ -116,14 +116,14 @@ START_TEST(test_print_ptests)

        head = ptest_list_alloc();
        ck_assert(print_ptests(head, fp) == 1);
-       ptest_list_free_all(head);
+       ptest_list_free_all(&head);
        line = fgets(line_buf, PRINT_PTEST_BUF_SIZE, fp);
        ck_assert(line != NULL);
        ck_assert(strcmp(line, PRINT_PTESTS_NOT_FOUND) == 0);

        head = get_available_ptests(opts_directory, 1);
        ck_assert(print_ptests(head, fp) == 0);
-       ptest_list_free_all(head);
+       ptest_list_free_all(&head);
        line = fgets(line_buf, PRINT_PTEST_BUF_SIZE, fp);
        ck_assert(line != NULL);
        ck_assert(strcmp(line, PRINT_PTESTS_AVAILABLE) == 0);
@@ -144,7 +144,7 @@ END_TEST
 START_TEST(test_filter_ptests)
 {
        struct ptest_list *head = get_available_ptests(opts_directory, 1);
-       struct ptest_list *head_new;
+       struct ptest_list *head_new = NULL;
        char *ptest_not_exists[] = {
                "glib",
        };
@@ -161,8 +161,8 @@ START_TEST(test_filter_ptests)
        ck_assert(head_new != NULL);
        ck_assert(ptest_list_length(head_new) == 3);

-       ptest_list_free_all(head);
-       ptest_list_free_all(head_new);
+       ptest_list_free_all(&head);
+       ptest_list_free_all(&head_new);
 }
 END_TEST

@@ -191,7 +191,7 @@ START_TEST(test_run_ptests)

        rc = run_ptests(head, opts, "test_run_ptests", fp_stdout, fp_stderr);
        ck_assert(rc == 0);
-       ptest_list_free_all(head);
+       ptest_list_free_all(&head);

        fclose(fp_stdout);
        free(buf_stdout);
@@ -227,7 +227,7 @@ START_TEST(test_run_timeout_duration_ptest)

        test_ptest_expected_failure(head, timeout, "hang", search_for_timeout_and_duration);

-       ptest_list_free_all(head);
+       ptest_list_free_all(&head);
 }
 END_TEST

@@ -255,7 +255,7 @@ START_TEST(test_run_fail_ptest)

        test_ptest_expected_failure(head, timeout, "fail", search_for_fail);

-       ptest_list_free_all(head);
+       ptest_list_free_all(&head);
 }
 END_TEST

@@ -354,7 +354,7 @@ test_ptest_expected_failure(struct ptest_list *head, const int timeout, char *pr
                        fp_stdout
                );

-               PTEST_LIST_FREE_ALL_CLEAN(filtered);
+               ptest_list_free_all(&filtered);
        }

        fclose(fp_stdout);
diff --git a/utils.c b/utils.c
index a23679a..4737bcd 100644
--- a/utils.c
+++ b/utils.c
@@ -110,7 +110,7 @@ get_ptest_file(char **ptest_file, struct stat *st_buf, const char *main_dir,
 struct ptest_list *
 get_available_ptests(const char *dir, int global_timeout)
 {
-       struct ptest_list *head;
+       struct ptest_list *head = NULL;
        struct stat st_buf;

        int n, i;
@@ -212,7 +212,7 @@ get_available_ptests(const char *dir, int global_timeout)
                free(namelist);

                if (fail) {
-                       PTEST_LIST_FREE_ALL_CLEAN(head);
+                       ptest_list_free_all(&head);
                        errno = saved_errno;
                        break;
                }
@@ -282,7 +282,7 @@ filter_ptests(struct ptest_list *head, char **ptests, int ptest_num)
                }

                if (fail) {
-                       PTEST_LIST_FREE_ALL_CLEAN(head_new);
+                       ptest_list_free_all(&head_new);
                        errno = saved_errno;
                }
        } while (0);
--
2.20.1




Join yocto@lists.yoctoproject.org to automatically receive all group messages.