Re: [PATCH] cgroup.c: Some 'hlist_head' function fixes.

From: Rakib Mullick
Date: Tue Aug 12 2008 - 08:12:52 EST


On 8/12/08, Paul Jackson <pj@xxxxxxx> 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 <pj@xxxxxxx> 1.940.382.4214
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/