lkml.org 
[lkml]   [2006]   [Jun]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] Clean up the bootmem allocator
From
Dave Hansen wrote:
> On Mon, 2006-06-26 at 15:11 +0200, Franck Bui-Huu wrote:
>> This patch does _only_ some cleanup in the bootmem allocator by:
> ...
>> include/linux/bootmem.h | 101 ++++++++++++++----------
>> mm/bootmem.c | 195 ++++++++++++++++++++++++++---------------------
>> 2 files changed, 167 insertions(+), 129 deletions(-)
>
> I'm always suspicious of "cleanups" which add more code than they remove ;)
>

it's mainly due to the "80 columns limit" changes

>> - following the kernel coding style conventions
>
> Above everything else, this probably needs to be in its very own patch,
> where it is trivially verifiable.
>
>> - using pfn/page conversion macros
>> - removing some not needed parentheses
>> - removing some useless included headers
>> - limiting to 80 column width
>
> In general, I think there is some good stuff in here. However, instead
> of concentrating on "kernel coding style conventions" and numeric (80
> column) guidelines, I really hope that people consider _readability_
> when modifying this code. I don't think this patch makes the code much
> more readable.
>

well, IMHO using pfn/page conversion macros makes the code more readable.

> That said, there are some nice helper function in here. Would you be
> able to break this patch up into maybe 10 or 15 smaller patches? I have
> the feeling it will be easier to find the good bits then.
>

ok I'll split that.

>> It also removes __init tags in the header file and hopefully make it
>> easier to read.
>
> I think I kinda like when these are present in headers. I usually
> stumble across the header declarations before I do the ones in the .c
> files, and it is always nice to see the header visually _matching_
> the .c file, and how the variable is intended to be used
>

see Andrew's comments...

Franck

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2006-06-27 10:21    [W:0.092 / U:0.228 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site