Re: [Qemu-devel] [PATCH 3/4] Multiboot support v2

[Alexander Graf <agraf@suse.de>]

On 18.06.2009, at 11:56, Avi Kivity wrote:

> On 06/17/2009 07:41 PM, Alexander Graf wrote:
>> This patch implements support for Multiboot on x86 for -kernel.
>> Multiboot is a "new" approach to get rid of different bootloaders,  
>> providing
>> a unified interface for the kernel. It supports command line  
>> options and
>> kernel modules.
>>
>> The two probably best known projects using multiboot are Xen and  
>> GNU Hurd.
>>
>> This implementation should be mostly feature-complete. It is  
>> missing VBE
>> extensions, but as no system uses them currently it does not really  
>> hurt.
>>
>> To use multiboot, specify the kernel as -kernel option. Modules  
>> should be given
>> as -initrd options, seperated by a comma (,). -append also works.
>>
>> Please bear in mind that grub also does gzip decompression, which  
>> qemu does
>> not do yet. To run existing images, please ungzip them first.
>>
>> The guest multiboot loader code is implemented as option rom using  
>> int 19.
>> Parts of the work are based on efforts by Rene Rebe, who originally  
>> ported
>> my code to int 19.
>>
>
> This will be very useful for running test cases.
>
>> +    /* Ok, let's see if it is a multiboot image.
>> +       The header is 12x32bit long, so the latest entry may be  
>> 8192 - 48. */
>> +    for(i = 0; i<  (8192 - 48); i += 4) {
>>
>
> Here (and in many other places in the patch) you have a control flow  
> keyword followed immediately by a parentheses, so it looks like a  
> function call.  Really reduced readability IMO.

Looking through pc.c I found for mostly without space, but if with  
immediate space after it.

Wouldn't it be more useful to write some automatic formatting script  
git would run on patches that go in? Maybe people using tabs in the  
wrong areas of the code would be able to submit patches by then too ;-).

IMHO there is no real "qemu coding style" atm anyway - each maintainer  
has his own and uses that for its subtree. It'd be a lot more helpful  
to start addressing things at the root of the problems.

Alex