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 <[email protected]> wrote:

> From: Adrian Freihofer <[email protected]>
>
> 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 <[email protected]>
> ---
>  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
>
>
> 
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#54217): https://lists.yoctoproject.org/g/yocto/message/54217
Mute This Topic: https://lists.yoctoproject.org/mt/84352909/21656
Group Owner: [email protected]
Unsubscribe: https://lists.yoctoproject.org/g/yocto/unsub 
[[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to