Messages in this thread | | | Date | Tue, 12 Aug 2008 18:12:37 +0600 | From | "Rakib Mullick" <> | Subject | Re: [PATCH] cgroup.c: Some 'hlist_head' function fixes. |
| |
On 8/12/08, Paul Jackson <pj@sgi.com> 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@sgi.com> 1.940.382.4214 >
| |