Re: [PATCH 6/9] Cleanup kmem_cache_create()

[Matthew Dobson <colpatch@us.ibm.com>]

Ingo Oeser wrote:
> On Friday 11 November 2005 01:04, Matthew Dobson wrote:
> 
>>- if (size < 4096 || fls(size - 1) == fls(size - 1 + 3 * BYTES_PER_WORD))
>>+ if (size < RED_ZONE_LIMIT ||
>>+	fls(size - 1) == fls(size - 1 + 3 * BYTES_PER_WORD))
>>             flags |= SLAB_RED_ZONE|SLAB_STORE_USER;
> 
> 
> I would suggest sth. like
> 
> if (size < RED_TONE_LIMIT
>     || fls(size - 1) = fls(size - 1 + 3 * BYTES_PER_WORD))
> 	flags |= SLAB_RED_ZONE | SLAB_STORE_USER
> 
> 
> Reason: A binary operator in front is a huge hint 
> 	that this is a continued line.
> 
> Just compare when you go to a store next time.
> 
> 	1
> +	2
> -	3
> *	4
> 
> is much more readable then
> 
> 1	+
> 2	-
> 3	*
> 4
> 
> right?
> 
> 
> Regards
> 
> Ingo Oeser

You make a very good point.  It's not the way that I'm used to writing
multi-line ifs, but it is a bit more readable.  However, since the rest of
the multi-line ifs in the file have the binary operator at the end of the
first line, I'm inclined to leave it the way it is for consistency.  If
anyone really feels strongly, I could come up with a patch for the whole
file...?

Thanks for the feedback!

-Matt