Re: [Minios-devel] [UNIKRAFT PATCH 1/2] plat/*: Add memory region names

[Florian Schmidt <Florian.Schmidt@xxxxxxxxx>]

On 01/26/2018 04:23 PM, Simon Kuenzer wrote:
[...]
> +#if UKPLAT_MEMRNAME
> +       const char *name;
> +#endif
[and many times below]

I'm not a huge fan of having ifdef guards everywhere, but if we want 
this dependent on a config option, I also don't see any better way.

> diff --git a/lib/ukboot/boot.c b/lib/ukboot/boot.c
> index 5158f32..acbae6e 100644
> --- a/lib/ukboot/boot.c
> +++ b/lib/ukboot/boot.c
> @@ -142,17 +142,31 @@ void ukplat_entry(int argc, char *argv[])
>   
>   			if ((md.flags & UKPLAT_MEMRF_ALLOCATABLE)
>                             != UKPLAT_MEMRF_ALLOCATABLE) {
> +#if UKPLAT_MEMRNAME
> +                               uk_printd(DLVL_EXTRA, "Skip memory region %d: %p - 
> %p (flags: 0x%02x, name: %s)\n",
> +                                         i, md.base, (void *)((size_t)md.base
> +                                                              + md.len),
> +                                         md.flags, md.name);
> +#else
>                                 uk_printd(DLVL_EXTRA, "Skip memory region %d: %p - 
> %p (flags: 0x%02x)\n",
>                                           i, md.base, (void *)((size_t)md.base
>                                                                + md.len),
>                                           md.flags);
> +#endif
>                                 continue;
>                         }
>   
> +#if UKPLAT_MEMRNAME
> +                       uk_printd(DLVL_EXTRA, "Try  memory region %d: %p - %p 
> (flags: 0x%02x, name: %s)...\n",
> +                                 i, md.base, (void *)((size_t)md.base
> +                                                      + md.len),
> +                                 md.flags, md.name);
> +#else
>                         uk_printd(DLVL_EXTRA, "Try  memory region %d: %p - %p 
> (flags: 0x%02x)...\n",
>                                   i, md.base, (void *)((size_t)md.base
>                                                        + md.len),
>                                   md.flags);
> +#endif
>                         /* try to use memory region to initialize allocator
>                          * if it fails, we will try  again with the next region.
>                          * As soon we have an allocator, we simply add every
If you see this kind of printing being used in more than those two 
places in the future, I'd prefer encapsulating it a function that deals 
with the ifdef.

But apart from these comments:
Reviewed-by: Florian Schmidt <florian.schmidt@xxxxxxxxx>



_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel