[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 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.
Additionally, I have improved comment seen above in patch #16 but it is much
softer (no shouting) as we agreed earlier.

Daniel

_______________________________________________
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®.