[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH for-xen-4.5 v3 02/16] x86/boot/reloc: Move typedef and include to beginning of file



>>> On 10.10.14 at 15:17, <daniel.kiper@xxxxxxxxxx> wrote:
> On Fri, Oct 10, 2014 at 09:50:12AM +0100, Jan Beulich wrote:
>> >>> On 08.10.14 at 19:52, <daniel.kiper@xxxxxxxxxx> wrote:
>> > --- a/xen/arch/x86/boot/reloc.c
>> > +++ b/xen/arch/x86/boot/reloc.c
>> > @@ -10,6 +10,10 @@
>> >   *    Keir Fraser <keir@xxxxxxx>
>> >   */
>> >
>> > +typedef unsigned int u32;
>> > +
>> > +#include "../../../include/xen/multiboot.h"
>> > +
>> >  /* entered with %eax = BOOT_TRAMPOLINE */
>> >  asm (
>> >      "    .text                         \n"
>> > @@ -30,9 +34,6 @@ asm (
>> >      "    .long 0                       \n"
>> >      );
>> >
>> > -typedef unsigned int u32;
>> > -#include "../../../include/xen/multiboot.h"
>> > -
>> >  static void *reloc_mbi_struct(void *old, unsigned int bytes)
>> >  {
>> >      void *new;
>>
>> In an earlier version you added some shouting warning comments to
>> point out where the entry point here is. Without your adjustment it
>> was right at the top of the file, making it rather obvious. I don't see
>> what the adjustment you do here is good for, and you also don't
>> say so (description is missing altogether).
> 
> Personally I think that all includes and definitions/declarations should be
> at the beginning of file (if it is possible) and separated from the "real 
> code".
> This makes files more readable. So, that is why I am doing this here.

While generally I agree, I think there are always reasons for exceptions.
And this is one of them.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.