Re: [PATCH v5 09/16] x86: add multiboot2 protocol support

From: Daniel Kiper <daniel.kiper@oracle.com>
To: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: jgross@suse.com, sstabellini@kernel.org, cardoe@cardoe.com,
	pgnet.dev@gmail.com, ning.sun@intel.com, david.vrabel@citrix.com,
	jbeulich@suse.com, xen-devel@lists.xenproject.org,
	qiaowei.ren@intel.com, gang.wei@intel.com, fu.wei@linaro.org
Subject: Re: [PATCH v5 09/16] x86: add multiboot2 protocol support
Date: Wed, 31 Aug 2016 23:01:03 +0200	[thread overview]
Message-ID: <20160831210103.GK5364@olila.local.net-space.pl> (raw)
In-Reply-To: <2fbf81bb-c282-858c-bd92-5b31cd727f4c@citrix.com>

On Wed, Aug 31, 2016 at 09:18:39PM +0100, Andrew Cooper wrote:
> On 19/08/2016 23:43, Daniel Kiper wrote:
> > @@ -106,3 +121,122 @@ multiboot_info_t __stdcall *reloc(u32 mbi_in, u32 trampoline)
> >
> >      return mbi_out;
> >  }
> > +
> > +static multiboot_info_t *mbi2_mbi(u32 mbi_in)
> > +{
> > +    const multiboot2_memory_map_t *mmap_src;
> > +    const multiboot2_tag_t *tag;
> > +    module_t *mbi_out_mods = NULL;
> > +    memory_map_t *mmap_dst;
> > +    multiboot_info_t *mbi_out;
> > +    u32 ptr;
> > +    unsigned int i, mod_idx = 0;
> > +
> > +    ptr = alloc_mem(sizeof(*mbi_out));
> > +    mbi_out = (multiboot_info_t *)ptr;
>
> It occurs to me that a lot of this code would be more simple if you also
> pulled the
>
> #define _p(val) ((void *)(unsigned long)val)
>
> in from the usual header files.  It would shorten a lot of the
> (multiboot_info_t *) casting, and reduce most multi-line statements to
> single lines.
>
> Can I also suggest using:

[...]

Make sense for me. I will try to get all your comments into account.

Thanks a lot,

Daniel

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