lib: list_sort_test(): simplify and harden cleanup
authorRasmus Villemoes <linux@rasmusvillemoes.dk>
Wed, 6 Aug 2014 23:09:42 +0000 (16:09 -0700)
committerLinus Torvalds <torvalds@linux-foundation.org>
Thu, 7 Aug 2014 01:01:25 +0000 (18:01 -0700)
There is no reason to maintain the list structure while freeing the
debug elements.  Aside from the redundant pointer manipulations, it is
also inefficient from a locality-of-reference viewpoint, since they are
visited in a random order (wrt.  the order they were allocated).
Furthermore, if we jumped to exit: after detecting list corruption, it
is actually dangerous.

So just free the elements in the order they were allocated, using the
backing array elts.  Allocate that using kcalloc(), so that if
allocation of one of the debug element fails, we just end up calling
kfree(NULL) for the trailing elements.

Minor details: Use sizeof(*elts) instead of sizeof(void *), and return
err immediately when allocation of elts fails, to avoid introducing
another label just before the final return statement.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Cc: Don Mullis <don.mullis@gmail.com>
Cc: Dave Chinner <david@fromorbit.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
lib/list_sort.c

index fbdbc867b252dd7dd1ffaf79c1a2a58d31461d70..a34c78c30d56615d0eb8334310772178fd328a65 100644 (file)
@@ -209,16 +209,16 @@ static int __init list_sort_test(void)
 {
        int i, count = 1, err = -ENOMEM;
        struct debug_el *el;
-       struct list_head *cur, *tmp;
+       struct list_head *cur;
        LIST_HEAD(head);
 
        printk(KERN_DEBUG "list_sort_test: start testing list_sort()\n");
 
-       elts = kmalloc(sizeof(void *) * TEST_LIST_LEN, GFP_KERNEL);
+       elts = kcalloc(TEST_LIST_LEN, sizeof(*elts), GFP_KERNEL);
        if (!elts) {
                printk(KERN_ERR "list_sort_test: error: cannot allocate "
                                "memory\n");
-               goto exit;
+               return err;
        }
 
        for (i = 0; i < TEST_LIST_LEN; i++) {
@@ -286,11 +286,9 @@ static int __init list_sort_test(void)
 
        err = 0;
 exit:
+       for (i = 0; i < TEST_LIST_LEN; i++)
+               kfree(elts[i]);
        kfree(elts);
-       list_for_each_safe(cur, tmp, &head) {
-               list_del(cur);
-               kfree(container_of(cur, struct debug_el, list));
-       }
        return err;
 }
 module_init(list_sort_test);