[PATCH] rhashtable: initialize all rhashtable walker members

[PATCH] rhashtable: initialize all rhashtable walker members

[Sasha Levin]

Commit "rhashtable: Introduce rhashtable_walk_*" forgot to initialize the
members of struct rhashtable_walker after allocating it, which caused
an undefined value for 'resize' which is used later on.

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 lib/rhashtable.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index 9cc4c4a..030484c 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -894,6 +894,9 @@ int rhashtable_walk_init(struct rhashtable *ht, struct rhashtable_iter *iter)
 	if (!iter->walker)
 		return -ENOMEM;
 
+	INIT_LIST_HEAD(&iter->walker->list);
+	iter->walker->resize = false;
+
 	mutex_lock(&ht->mutex);
 	list_add(&iter->walker->list, &ht->walkers);
 	mutex_unlock(&ht->mutex);
-- 
1.7.10.4

Re: [PATCH] rhashtable: initialize all rhashtable walker members

[David Miller]

From: Sasha Levin <sasha.levin@oracle.com>
Date: Sat, 21 Feb 2015 15:55:11 -0500

> Commit "rhashtable: Introduce rhashtable_walk_*" forgot to initialize the
> members of struct rhashtable_walker after allocating it, which caused
> an undefined value for 'resize' which is used later on.
> 
> Signed-off-by: Sasha Levin <sasha.levin@oracle.com>

Commits should be referred to by SHA1 ID as well as the commit
header text (in parenthesis and double quotes).

Even better, refer to the commit using a "Fixes: " tag right before
your signoff.

Re: [PATCH] rhashtable: initialize all rhashtable walker members

[Sasha Levin]

On 02/22/2015 03:58 PM, David Miller wrote:
> From: Sasha Levin <sasha.levin@oracle.com>
> Date: Sat, 21 Feb 2015 15:55:11 -0500
> 
>> Commit "rhashtable: Introduce rhashtable_walk_*" forgot to initialize the
>> members of struct rhashtable_walker after allocating it, which caused
>> an undefined value for 'resize' which is used later on.
>>
>> Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
> 
> Commits should be referred to by SHA1 ID as well as the commit
> header text (in parenthesis and double quotes).

It wasn't (isn't?) in Linus' tree yet, so I didn't want to refer to
it by a SHA1 ID which would change.

> Even better, refer to the commit using a "Fixes: " tag right before
> your signoff.

Ok, should I resend?


Thanks,
Sasha

Re: [PATCH] rhashtable: initialize all rhashtable walker members

[David Miller]

From: Sasha Levin <sasha.levin@oracle.com>
Date: Sun, 22 Feb 2015 16:28:05 -0500

> On 02/22/2015 03:58 PM, David Miller wrote:
>> From: Sasha Levin <sasha.levin@oracle.com>
>> Date: Sat, 21 Feb 2015 15:55:11 -0500
>> 
>>> Commit "rhashtable: Introduce rhashtable_walk_*" forgot to initialize the
>>> members of struct rhashtable_walker after allocating it, which caused
>>> an undefined value for 'resize' which is used later on.
>>>
>>> Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
>> 
>> Commits should be referred to by SHA1 ID as well as the commit
>> header text (in parenthesis and double quotes).
> 
> It wasn't (isn't?) in Linus' tree yet, so I didn't want to refer to
> it by a SHA1 ID which would change.

Commit IDs in my tree _NEVER_ change.

Re: [PATCH] rhashtable: initialize all rhashtable walker members

[Daniel Borkmann]

On 02/21/2015 09:55 PM, Sasha Levin wrote:
> Commit "rhashtable: Introduce rhashtable_walk_*" forgot to initialize the
> members of struct rhashtable_walker after allocating it, which caused
> an undefined value for 'resize' which is used later on.
>
> Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
> ---
>   lib/rhashtable.c |    3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/lib/rhashtable.c b/lib/rhashtable.c
> index 9cc4c4a..030484c 100644
> --- a/lib/rhashtable.c
> +++ b/lib/rhashtable.c
> @@ -894,6 +894,9 @@ int rhashtable_walk_init(struct rhashtable *ht, struct rhashtable_iter *iter)
>   	if (!iter->walker)
>   		return -ENOMEM;
>
> +	INIT_LIST_HEAD(&iter->walker->list);
> +	iter->walker->resize = false;
> +

The change seems fine to me, although the INIT_LIST_HEAD() unnecessary
due to the below list_add()?

Anyway, setting resize to false is definitely correct. In practice this
shouldn't cause much issue though as rhashtable_walk_start() would only
reset iterator meta data and set resize to false, but lets fix it.

>   	mutex_lock(&ht->mutex);
>   	list_add(&iter->walker->list, &ht->walkers);
>   	mutex_unlock(&ht->mutex);

Thanks,
Daniel