Re: [dpdk-dev] [PATCH v6 03/10] windows/eal: translate Windows errors to errno-style errors
From: Dmitry Kozlyuk <hidden>
Date: 2021-04-29 00:50:47
2021-04-02 18:39 (UTC-0700), Narcisa Ana Maria Vasile:
From: Narcisa Vasile <redacted> Add function to translate Windows error codes to errno-style error codes. Signed-off-by: Narcisa Vasile <redacted>
Commit topic should be "eal/windows", not "windows/eal".
quoted hunk ↗ jump to hunk
--- lib/librte_eal/include/rte_thread.h | 5 +- lib/librte_eal/windows/rte_thread.c | 75 ++++++++++++++++++++++------- 2 files changed, 60 insertions(+), 20 deletions(-)diff --git a/lib/librte_eal/include/rte_thread.h b/lib/librte_eal/include/rte_thread.h index bfdd8e1b1..2d7b3bc05 100644 --- a/lib/librte_eal/include/rte_thread.h +++ b/lib/librte_eal/include/rte_thread.h@@ -221,9 +221,8 @@ int rte_thread_value_set(rte_thread_key key, const void *value); * * @return * On success, value data pointer (can also be NULL). - * On failure, NULL and an error number is set in rte_errno. - * rte_errno can be: EINVAL - Invalid parameter passed. - * ENOEXEC - Specific OS error. + * On failure, NULL and a positive error number is set in rte_errno. + * */ __rte_experimental void *rte_thread_value_get(rte_thread_key key);diff --git a/lib/librte_eal/windows/rte_thread.c b/lib/librte_eal/windows/rte_thread.c index b29336cbd..ecd2f810f 100644 --- a/lib/librte_eal/windows/rte_thread.c +++ b/lib/librte_eal/windows/rte_thread.c@@ -12,6 +12,51 @@ struct eal_tls_key { DWORD thread_index; }; +/* Translates the most common error codes related to threads */ +static int rte_thread_translate_win32_error(void)
Return type should be on a separate line.
+{
+ DWORD error = 0;
+
+ error = GetLastError();You could declare "error" and assign it in one statement. Don't initialize variables if there's no good initial value: it prevents compiler to issue a warning if the variable is not assigned a meaningful value afterwards.
+
+ 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;
+
+ default:
+ return EINVAL;
+ }Default branch is redundant at the end of function.
quoted hunk ↗ jump to hunk
+ + return EINVAL; +} + rte_thread_t rte_thread_self(void) {@@ -87,15 +132,13 @@ rte_thread_key_create(rte_thread_key *key, *key = malloc(sizeof(**key)); if ((*key) == NULL) { RTE_LOG(DEBUG, EAL, "Cannot allocate TLS key.\n"); - rte_errno = ENOMEM; - return -1; + return ENOMEM; } (*key)->thread_index = TlsAlloc(); if ((*key)->thread_index == TLS_OUT_OF_INDEXES) { RTE_LOG_WIN32_ERR("TlsAlloc()"); free(*key); - rte_errno = ENOEXEC; - return -1; + return rte_thread_translate_win32_error();
Logging above can overwrite GetLastError() value. I suggest splitting rte_thread_translate_win32_error() into translation part for cases when you have error number already, and a wrapper that calls GetLastError() to shorten calling code. Same applies below in this file. [...]