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

Re: [Minios-devel] [UNIKRAFT PATCH 3/4] lib/ukcontext: Adapt ucontext.h



Hello Costin,


thank you for the review. Please see inline.




From: Costin Lupu <costin.lup@xxxxxxxxx>
Sent: Thursday, August 22, 2019 6:51 PM
To: Charalampos Mainas; minios-devel@xxxxxxxxxxxxx
Subject: Re: [Minios-devel] [UNIKRAFT PATCH 3/4] lib/ukcontext: Adapt ucontext.h
 
Hi Charalampos,

Please see my comments inline.

On 8/22/19 7:06 PM, Charalampos Mainas wrote:
> Signed-off by: Charalampos Mainas <charalampos.mainas@xxxxxxxxx>

s/Signed-off by/Signed-off-by/g

> ---
>  lib/ukucontext/include/ucontext.h | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/lib/ukucontext/include/ucontext.h b/lib/ukucontext/include/ucontext.h
> index eb83ce24..07837882 100644
> --- a/lib/ukucontext/include/ucontext.h
> +++ b/lib/ukucontext/include/ucontext.h
> @@ -59,6 +59,9 @@ typedef struct {
>  } mcontext_t;
>  #endif

> +// Conflict with sigset_t from newlib. Using same definition as in musl
> +typedef struct { unsigned long __bits[128/sizeof(long)]; } uco_sigset_t;
> +
>  struct sigaltstack {
>        void *ss_sp;
>        int ss_flags;
> @@ -68,9 +71,9 @@ struct sigaltstack {
>  typedef struct __ucontext {
>        unsigned long uc_flags;
>        struct __ucontext *uc_link;
> -     stack_t uc_stack;
> +     struct sigaltstack uc_stack;

Why don't we add the stack_t type definition instead?

stack_t  is defined as struct sigaltstack in include/signal.h (in musl) and has the same
definition in newlib.I thought it would be better to stick with how musl defines it and
do not depend on newlib.

>        mcontext_t uc_mcontext;
> -     sigset_t uc_sigmask;
> +     uco_sigset_t uc_sigmask;

Isn't this supposed to be actually inline with the sigset_t definition?
What don't we use the newlib's type definition?

Musl defines sigset_t in a different way than newlib. (in newlib is defined as unsigned long
and in musl as above) However glibc has the same definition with newlib, so we should follow
newlib's definition.

>        unsigned long __fpregs_mem[64];
>  } ucontext_t;

>
_______________________________________________
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®.