[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 02/18] x86/boot/reloc: create generic alloc and copy function
From: |
Jan Beulich |
Subject: |
Re: [PATCH 02/18] x86/boot/reloc: create generic alloc and copy functions |
Date: |
Tue, 03 Feb 2015 10:13:59 +0000 |
>>> On 30.01.15 at 18:54, <address@hidden> 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
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [PATCH 02/18] x86/boot/reloc: create generic alloc and copy functions,
Jan Beulich <=