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

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



Hi Sharan,


On 08/24/2018 02:45 PM, Sharan Santhanam wrote:
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.

Actually, after refactoring the code, we don't even need thi definition anywhere, so I removed it. (sigset is only used in the linux platform code, which needs to provide its own k_sigset_t anyway to ensure ABI compatibility with the Linux kernel.) Once we need functions that actually use sigset_t, we can choose what data type feels best.

+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.

Same as above.

+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?

Same as above.

+#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

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