Re: [Xen-devel] [PATCH 02/18] x86/boot/reloc: create generic alloc and copy functions

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 30.01.15 at 18:54, <daniel.kiper@xxxxxxxxxx> 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"
>  
> -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 = old; *p != '\0'; p++ )
> +
> +    if ( src == 0 )
> +        return 0;
> +
> +    for ( p = (char *)src; *p != '\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 = reloc_mbi_struct(mbi_old, sizeof(*mbi));
> +    multiboot_info_t *mbi = (multiboot_info_t *)copy_struct((u32)mbi_old, 
> sizeof(*mbi));

Please get rid of the cast on the first argument by converting
reloc()'s parameter type accordingly.

>      int i;
>  
>      if ( mbi->flags & MBI_CMDLINE )
> -        mbi->cmdline = (u32)reloc_mbi_string((char *)mbi->cmdline);
> +        mbi->cmdline = copy_string(mbi->cmdline);
>  
>      if ( mbi->flags & MBI_MODULES )
>      {
> -        module_t *mods = reloc_mbi_struct(
> -            (module_t *)mbi->mods_addr, mbi->mods_count * sizeof(module_t));
> +        module_t *mods = (module_t *)copy_struct(
> +            mbi->mods_addr, mbi->mods_count * sizeof(module_t));
>  
>          mbi->mods_addr = (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


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