Hello guys, the following patch emphasizes on two things:
1. We can carry out the following function with one variable.
2. As hash_long returns with unsigned long we need a unsigned long to
handle this.
If anything else, please notice.
Thanks.
Signed-off-by: Md.Rakib H. Mullick ([email protected])
--- linux-2.6.27-rc2.orig/kernel/cgroup.c 2008-08-06 16:23:26.000000000 +0600
+++ linux-2.6.27-rc2/kernel/cgroup.c 2008-08-08 19:06:53.000000000 +0600
@@ -200,17 +200,16 @@ static struct hlist_head css_set_table[C
static struct hlist_head *css_set_hash(struct cgroup_subsys_state *css[])
{
- int i;
- int index;
- unsigned long tmp = 0UL;
+ unsigned long tmp = 0UL , i;
for (i = 0; i < CGROUP_SUBSYS_COUNT; i++)
tmp += (unsigned long)css[i];
tmp = (tmp >> 16) ^ tmp;
- index = hash_long(tmp, CSS_SET_HASH_BITS);
+ i = 0;
+ i = hash_long(tmp, CSS_SET_HASH_BITS);
- return &css_set_table[index];
+ return &css_set_table[i];
}
/* We don't maintain the lists running through each css_set to its
Rabik wrote:
> + i = 0;
> + i = hash_long(tmp, CSS_SET_HASH_BITS);
That "i = 0;" looks unnecessary to me, given
that i is unconditionally set again, on the
very next line.
--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <[email protected]> 1.940.382.4214
On 8/12/08, Paul Jackson <[email protected]> wrote:
> > Is it [text size] the only criteria to judge this patch ?
>
> No - not the only criteria, as the patch combines a couple of
> changes.
>
> > What about the use of "unsigned long", instead of int.
>
> I had missed that change, even though you had explicitly
>
> described it in your patch comment, when you wrote:
>
> 2. As hash_long returns with unsigned long we need a unsigned long
>
>
> How about just casting the hash_long() result to int:
>
> index = (int)hash_long(tmp, CSS_SET_HASH_BITS);
Yes, it looks good.
>
> Since we are using this 'index' to index an array,
> it had better fit in an 'int', which indeed it does
> as CSS_SET_HASH_BITS is 7, which constrains the output
> of hash_long to [0 .. 2^7-1], that is between 0 and 127.
>
> However ... looking around the kernel, I see that most other
> uses of hash_long(), except in cases where the second argument
> (bit size) might actually exceed 32 bits, either directly
> index some array with the result, or else assign the result
> to a temporary 'int'.
>
> And the compiler does not complain that we're assigning a
> long to an int.
>
> So ... what's the problem?
Yes, maybe your right. Ok, I'll go through the code again. If it's
good then I'm happy.
>
> I see nothing in this patch of value.
>
> Am I missing something?
>
> -----
>
> I just noticed that you had dropped the other recipients
> from this email thread, a couple of replies ago. My
> preference would have been to have this discussion in
> public. I prefer not to drop people from CC lists on
> email threads.
Yes, I've noticed it too. I just forgot to do that. Actually, when I
reply to thread, I just think about that one. This could be a reason
for missing.
Thanks.
>
>
> --
>
> I won't rest till it's the best ...
> Programmer, Linux Scalability
> Paul Jackson <[email protected]> 1.940.382.4214
>