Re: [dpdk-dev] [PATCH v16 3/9] eal/windows: translate Windows errors to errno-style errors

On Tue, Oct 12, 2021 at 06:16:19PM +0200, Thomas Monjalon wrote:

09/10/2021 09:41, Narcisa Ana Maria Vasile:

quoted

From: Narcisa Vasile <redacted>

Add function to translate Windows error codes to
errno-style error codes. The possible return values are chosen
so that we have as much semantical compatibility between platforms as
possible.

Signed-off-by: Narcisa Vasile <redacted>
---
 lib/eal/common/rte_thread.c  |  6 +--
 lib/eal/include/rte_thread.h |  5 +-
 lib/eal/windows/rte_thread.c | 95 +++++++++++++++++++++++++++---------
 3 files changed, 76 insertions(+), 30 deletions(-)

diff --git a/lib/eal/common/rte_thread.c b/lib/eal/common/rte_thread.c
index e1a4d7eae4..27ad1c7eb0 100644
--- a/lib/eal/common/rte_thread.c
+++ b/lib/eal/common/rte_thread.c

@@ -47,7 +47,7 @@ rte_thread_attr_init(rte_thread_attr_t *attr)
 
 int
 rte_thread_attr_set_affinity(rte_thread_attr_t *thread_attr,
-			     rte_cpuset_t *cpuset)
+		rte_cpuset_t *cpuset)
 {
 	RTE_VERIFY(thread_attr != NULL);
 	RTE_VERIFY(cpuset != NULL);

@@ -59,7 +59,7 @@ rte_thread_attr_set_affinity(rte_thread_attr_t *thread_attr,
 
 int
 rte_thread_attr_get_affinity(rte_thread_attr_t *thread_attr,
-			     rte_cpuset_t *cpuset)
+		rte_cpuset_t *cpuset)
 {
 	RTE_VERIFY(thread_attr != NULL);
 	RTE_VERIFY(cpuset != NULL);

@@ -71,7 +71,7 @@ rte_thread_attr_get_affinity(rte_thread_attr_t *thread_attr,
 
 int
 rte_thread_attr_set_priority(rte_thread_attr_t *thread_attr,
-			     enum rte_thread_priority priority)
+		enum rte_thread_priority priority)

Above are unrelated changes.

  Thanks, will fix!

quoted

--- a/lib/eal/windows/rte_thread.c
+++ b/lib/eal/windows/rte_thread.c

@@ -13,6 +13,54 @@ struct eal_tls_key {
 	DWORD thread_index;
 };
 
+/* Translates the most common error codes related to threads */
+static int
+thread_translate_win32_error(DWORD error)

So you decide to adopt POSIX error codes for the DPDK API. OK

quoted

+{
+	switch (error) {
+	case ERROR_SUCCESS:
+		return 0;
+
+	case ERROR_INVALID_PARAMETER:
+		return EINVAL;
+
+	case ERROR_INVALID_HANDLE:
+		return EFAULT;
+
+	case ERROR_NOT_ENOUGH_MEMORY:
+	/* FALLTHROUGH */
+	case ERROR_NO_SYSTEM_RESOURCES:
+		return ENOMEM;
+
+	case ERROR_PRIVILEGE_NOT_HELD:
+	/* FALLTHROUGH */
+	case ERROR_ACCESS_DENIED:
+		return EACCES;
+
+	case ERROR_ALREADY_EXISTS:
+		return EEXIST;
+
+	case ERROR_POSSIBLE_DEADLOCK:
+		return EDEADLK;
+
+	case ERROR_INVALID_FUNCTION:
+	/* FALLTHROUGH */
+	case ERROR_CALL_NOT_IMPLEMENTED:
+		return ENOSYS;
+	}
+
+	return EINVAL;
+}

[...]

quoted

 rte_thread_key_create(rte_thread_key *key,
 		__rte_unused void (*destructor)(void *))
 {
+	int ret;
+
 	*key = malloc(sizeof(**key));
 	if ((*key) == NULL) {
 		RTE_LOG(DEBUG, EAL, "Cannot allocate TLS key.\n");
-		rte_errno = ENOMEM;
-		return -1;
+		return ENOMEM;
 	}

Why this change? rte_errno and negative error code are good.

  This error could have been handled using rte_errno and negative return,
  but for consistency, a positive error number is returned. As different platforms
  have different error codes, the approach here is to translate the Windows error
  to POSIX-style ones to have uniformity over the values returned. All functions
  in this thread module return the possible error through the return value.