Re: [Minios-devel] [UNIKRAFT PATCH v4 1/9] lib/nolibc: Add type definitions for timer support

[Sharan Santhanam <sharan.santhanam@xxxxxxxxx>]

Hello,

Please find my comment inline.

On 08/20/2018 01:21 PM, Florian Schmidt wrote:
> Add missing type definition for timer support with nolibc.
>
> Signed-off-by: Costin Lupu <costin.lupu@xxxxxxxxx>
> Signed-off-by: Florian Schmidt <florian.schmidt@xxxxxxxxx>
> ---
>   lib/nolibc/include/nolibc-internal/shareddefs.h | 15 +++++++++++++++
>   lib/nolibc/include/sys/select.h                 |  6 ++++++
>   lib/nolibc/include/sys/types.h                  |  4 ++++
>   3 files changed, 25 insertions(+)
>
> diff --git a/lib/nolibc/include/nolibc-internal/shareddefs.h 
> b/lib/nolibc/include/nolibc-internal/shareddefs.h
> index 9b81fab..8b953c2 100644
> --- a/lib/nolibc/include/nolibc-internal/shareddefs.h
> +++ b/lib/nolibc/include/nolibc-internal/shareddefs.h
> @@ -65,6 +65,11 @@ typedef __off off_t;
>   #define __DEFINED_off_t
>   #endif
>   
> +#if (defined __NEED_sigset_t && !defined __DEFINED_sigset_t)
According to this posix spec[1], sigset_t should be integer or a 
structure object.
> +typedef unsigned long sigset_t;
> +#define __DEFINED_sigset_t
> +#endif
> +
>   #if (defined __NEED_time_t && !defined __DEFINED_time_t)
>   typedef long time_t;
>   #define __DEFINED_time_t
> @@ -90,3 +95,13 @@ struct timespec {
>   };
>   #define __DEFINED_struct_timespec
>   #endif
> +
> +#if (defined __NEED_clockid_t && !defined __DEFINED_clockid_t)
clockid_t is defined as signed long. But since it is an identifier with 
a handful of values it might be fine.
> +typedef unsigned long clockid_t;
Should be __DEFINED_clockid_t instead of __DEFINED_clockid_d.
> +#define __DEFINED_clockid_d
> +#endif
> +
timer_t Posix specification does not specify the actual type 
information. But glibc, musl define it as (void *) and in the linux 
kernel it is defined as (int). We have defined it as unsigned long.
Why did we make this choice?
> +#if (defined __NEED_timer_t && !defined __DEFINED_timer_t)
> +typedef unsigned long timer_t;
> +#define __DEFINED_timer_t
> +#endif
> diff --git a/lib/nolibc/include/sys/select.h b/lib/nolibc/include/sys/select.h
> index e2bc70d..01ca905 100644
> --- a/lib/nolibc/include/sys/select.h
> +++ b/lib/nolibc/include/sys/select.h
> @@ -36,6 +36,12 @@
>   extern "C" {
>   #endif
>   
> +#define __NEED_time_t
> +#define __NEED_suseconds_t
> +#define __NEED_sigset_t
> +#define __NEED_struct_timespec
> +#include <nolibc-internal/shareddefs.h>
> +
>   typedef unsigned long __fd_mask;
>   
>   /*
> diff --git a/lib/nolibc/include/sys/types.h b/lib/nolibc/include/sys/types.h
> index 9908855..f2b6633 100644
> --- a/lib/nolibc/include/sys/types.h
> +++ b/lib/nolibc/include/sys/types.h
> @@ -46,6 +46,10 @@ extern "C" {
>   #define __NEED_size_t
>   #define __NEED_ssize_t
>   #define __NEED_off_t
> +#define __NEED_time_t
> +#define __NEED_suseconds_t
> +#define __NEED_clockid_t
> +#define __NEED_timer_t
>   #include <nolibc-internal/shareddefs.h>
>   
>   #ifdef __cplusplus

[1] http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/signal.h.html
[2] http://pubs.opengroup.org/onlinepubs/009695299/basedefs/sys/types.h.html

Thanks & Regards
Sharan

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