On Thu, May 18, 2017 at 11:35 AM, Leno Hou <[email protected]> wrote:
Commit message, please.
And I think you do introduce test cases first, then you update the
code. This way is naturally like things would be done.
> +++ b/lib/btree_test.c
Okay, this will be 5th in the pattern foo_bar_test.c :-)
I think it's okay since we have that pattern for tree implementations.
> @@ -0,0 +1,77 @@
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/btree.h>
> +#include <linux/types.h>
Alphabetical order, please.
> +
> +#define NODES 24
BTREE_TEST_...
> +
> +struct test_node {
> + u32 key;
> + u32 val;
> +};
> +
> +static struct btree_head32 bh;
> +static struct test_node nodes[NODES];
> +
> +static void init(void)
> +{
> + int i;
> +
> + for (i = 0; i < NODES; i++) {
> + nodes[i].key = i;
> + nodes[i].val = i;
> + }
> +}
> +static int __init btree_test_init(void)
> +{
> + u32 key = 0;
> + u32 *val = NULL;
Assignments look to me redundant. for_each should do that, shouldn't it?
> + int i, rc;
unsigned int i;
?
> +
> + pr_alert("btree testing\n");
We are using pr_info() / pr_warn() pair in the rest of test_* modules.
Please, follow.
E.g. here pr_info() is suitable.
> +
> + init();
> + rc = btree_init32(&bh);
> +
Move this empty line to be after init();.
> + if (rc)
> + pr_alert("Unable initialize btree memory\n");
> +
> + for (i = 0; i < NODES; i++) {
> + rc = btree_insert32(&bh,
> + nodes[i].key,
> + &nodes[i].val,
> + GFP_ATOMIC);
Is this necessary to be ATOMIC?
> +
> + if (rc)
> + pr_alert("Unable to insert key into btree\n");
> + }
> +
> + pr_alert("========================================\n");
> +
> + btree_for_each_safe32(&bh, key, val) {
> + pr_alert("val %d\n", *val);
> + }
> +
> + btree_remove32(&bh, 11);
> +
> + pr_alert("========================================\n");
> + btree_for_each_safe32(&bh, key, val) {
> + pr_alert("val %d\n", *val);
> + }
> +
> + return 0;
Consider to return -EINVAL if tests are failed. Thus, you need to
introduce actual testing function.
> +}
--
With Best Regards,
Andy Shevchenko
