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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
