[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Minios-devel] [UNIKRAFT PATCH] plat/xen: Fix memregion index into _libxenplat_mrd



Hi Florian,

Sure, I'm sending the more verbose v2 patch soon and I will implement the memregion enums later.

Thanks!
Cristian

On Fri, Apr 5, 2019 at 10:13 AM Florian Schmidt <Florian.Schmidt@xxxxxxxxx> wrote:
Hi Christian,

thanks for noticing this! This indeed seems like an oversight and was
most likely introduced in patch d1aae7129. Could you please make the
commit message a bit more verbose and mention the context of this? I'll
test this patch later today (my Xen test machine is currently down).

Also, as a second step, while this fixes the bug, this setup seems very
brittle, because whenever a new entry is added to the switch statement,
this number needs to be updated. Maybe a better solution would be to
create an memregion enum that has all the numbers as named entries
(which would also dispense with the "/* text */" comments, because the
code itself would be clearer), and a last entry called "count" or "last"
or something like that, which could then be used instead of this magic
number 5 or 7 or whatever it would be in the future.

Would you want to implement something like this?

Cheers,
Florian

On 4/5/19 7:29 AM, Cristian Banu wrote:
> Signed-off-by: Cristian Banu <cristb@xxxxxxxxx>
> ---
>   plat/xen/memory.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/plat/xen/memory.c b/plat/xen/memory.c
> index 4b3e94661a14..cb1c3552a0d9 100644
> --- a/plat/xen/memory.c
> +++ b/plat/xen/memory.c
> @@ -133,7 +133,7 @@ int ukplat_memregion_get(int i, struct ukplat_memregion_desc *m)
>   #endif
>                       return -1;
>               } else {
> -                     memcpy(m, &_libxenplat_mrd[i - 5], sizeof(*m));
> +                     memcpy(m, &_libxenplat_mrd[i - 7], sizeof(*m));
>               }
>               break;
>       }
>

--
Dr. Florian Schmidt
フローリアン・シュミット
Research Scientist,
Systems and Machine Learning Group
NEC Laboratories Europe
Kurfürsten-Anlage 36, D-69115 Heidelberg
Tel.     +49 (0)6221 4342-265
Fax:     +49 (0)6221 4342-155
e-mail:  florian.schmidt@xxxxxxxxx
============================================================
Registered at Amtsgericht Mannheim, Germany, HRB728558
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.