From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1YIaUl-0007Hp-3Q for mharc-grub-devel@gnu.org; Tue, 03 Feb 2015 05:14:11 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51423) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YIaUi-0007GF-C7 for grub-devel@gnu.org; Tue, 03 Feb 2015 05:14:09 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YIaUc-0004Si-KW for grub-devel@gnu.org; Tue, 03 Feb 2015 05:14:08 -0500 Received: from mail.emea.novell.com ([130.57.118.101]:47002) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YIaUc-0004SQ-CX for grub-devel@gnu.org; Tue, 03 Feb 2015 05:14:02 -0500 Received: from EMEA1-MTA by mail.emea.novell.com with Novell_GroupWise; Tue, 03 Feb 2015 10:14:00 +0000 Message-Id: <54D0AD77020000780005C4A8@mail.emea.novell.com> X-Mailer: Novell GroupWise Internet Agent 14.0.1 Date: Tue, 03 Feb 2015 10:13:59 +0000 From: "Jan Beulich" To: "Daniel Kiper" Subject: Re: [PATCH 02/18] x86/boot/reloc: create generic alloc and copy functions References: <1422640462-28103-1-git-send-email-daniel.kiper@oracle.com> <1422640462-28103-3-git-send-email-daniel.kiper@oracle.com> In-Reply-To: <1422640462-28103-3-git-send-email-daniel.kiper@oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Content-Disposition: inline X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 130.57.118.101 Cc: Juergen Gross , grub-devel@gnu.org, keir@xen.org, ian.campbell@citrix.com, stefano.stabellini@eu.citrix.com, andrew.cooper3@citrix.com, roy.franz@linaro.org, ning.sun@intel.com, david.vrabel@citrix.com, phcoder@gmail.com, xen-devel@lists.xenproject.org, qiaowei.ren@intel.com, richard.l.maliszewski@intel.com, gang.wei@intel.com, fu.wei@linaro.org X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list Reply-To: The development of GNU GRUB List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 03 Feb 2015 10:14:09 -0000 >>> On 30.01.15 at 18:54, wrote: > --- a/xen/arch/x86/boot/reloc.c > +++ b/xen/arch/x86/boot/reloc.c > @@ -33,9 +33,10 @@ asm ( > typedef unsigned int u32; > #include "../../../include/xen/multiboot.h" > =20 > -static void *reloc_mbi_struct(void *old, unsigned int bytes) > +static u32 alloc_struct(u32 bytes) The generalization calls for the naming to change. Especially with the use from copy_string(), both of the functions no longer deal with struct-s alone. Please name them ..._block() or some other more neutral way. > +static u32 copy_string(u32 src) > { > char *p; > - for ( p =3D old; *p !=3D '\0'; p++ ) > + > + if ( src =3D=3D 0 ) > + return 0; > + > + for ( p =3D (char *)src; *p !=3D '\0'; p++ ) > continue; > - return reloc_mbi_struct(old, p - old + 1); > + > + return copy_struct(src, p - (char *)src + 1); > } As few casts as possible please: You can get away with one if you type "p" u32. > multiboot_info_t *reloc(multiboot_info_t *mbi_old) > { > - multiboot_info_t *mbi =3D reloc_mbi_struct(mbi_old, sizeof(*mbi)); > + multiboot_info_t *mbi =3D (multiboot_info_t *)copy_struct((u32)mbi_o= ld, sizeof(*mbi)); Please get rid of the cast on the first argument by converting reloc()'s parameter type accordingly. > int i; > =20 > if ( mbi->flags & MBI_CMDLINE ) > - mbi->cmdline =3D (u32)reloc_mbi_string((char *)mbi->cmdline); > + mbi->cmdline =3D copy_string(mbi->cmdline); > =20 > if ( mbi->flags & MBI_MODULES ) > { > - module_t *mods =3D reloc_mbi_struct( > - (module_t *)mbi->mods_addr, mbi->mods_count * sizeof(module_= t)); > + module_t *mods =3D (module_t *)copy_struct( > + mbi->mods_addr, mbi->mods_count * sizeof(module_t)); > =20 > mbi->mods_addr =3D (u32)mods; And again you can get away with one less cast if you store the result of copy_struct() here and then assign "mods". Jan