[v2,1/4] eal/windows: translate Windows errors to errno-style errors
Checks
Commit Message
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 <navasile@microsoft.com>
Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
lib/eal/windows/rte_thread.c | 49 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 49 insertions(+)
Comments
Hi, Tyler
On 4/12/2022 3:43 AM, Tyler Retzlaff wrote:
> 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 <navasile@microsoft.com>
> Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> ---
> lib/eal/windows/rte_thread.c | 49 ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 49 insertions(+)
>
> diff --git a/lib/eal/windows/rte_thread.c b/lib/eal/windows/rte_thread.c
> index 667287c..c272018 100644
> --- a/lib/eal/windows/rte_thread.c
> +++ b/lib/eal/windows/rte_thread.c
> @@ -1,5 +1,6 @@
> /* SPDX-License-Identifier: BSD-3-Clause
> * Copyright 2021 Mellanox Technologies, Ltd
> + * Copyright (C) 2022 Microsoft Corporation
> */
>
> #include <rte_common.h>
> @@ -11,6 +12,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)
> +{
> + 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;
> +}
> +
Shouldn't we return all these error values as negative... as in -EINVAL,
-EFAULT etc. ?
ranjit m.
On Tue, Apr 12, 2022 at 10:26:27AM -0700, Menon, Ranjit wrote:
> Hi, Tyler
>
> On 4/12/2022 3:43 AM, Tyler Retzlaff wrote:
> >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 <navasile@microsoft.com>
> >Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> >---
> > lib/eal/windows/rte_thread.c | 49 ++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 49 insertions(+)
> >
> >diff --git a/lib/eal/windows/rte_thread.c b/lib/eal/windows/rte_thread.c
> >index 667287c..c272018 100644
> >--- a/lib/eal/windows/rte_thread.c
> >+++ b/lib/eal/windows/rte_thread.c
> >@@ -1,5 +1,6 @@
> > /* SPDX-License-Identifier: BSD-3-Clause
> > * Copyright 2021 Mellanox Technologies, Ltd
> >+ * Copyright (C) 2022 Microsoft Corporation
> > */
> > #include <rte_common.h>
> >@@ -11,6 +12,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)
> >+{
> >+ 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;
> >+}
> >+
>
> Shouldn't we return all these error values as negative... as in
> -EINVAL, -EFAULT etc. ?
no. since this is just a translation function it is up to the caller
what it will do with the translated value and any conversion to -errno.
since it is also used to store into rte_errno it would have been
confusing to see something like the following to reverse the sign back.
err = thread_translate_win32_error(win32err);
rte_errno = -err; // confusing
personally i don't like the -errno returns at all and this is just
another reason contributing to that opinion. you can refer to my
previous mailing list posts on the subject.
>
>
> ranjit m.
On Tue, Apr 12, 2022 at 12:44 PM Tyler Retzlaff
<roretzla@linux.microsoft.com> wrote:
>
> 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 <navasile@microsoft.com>
> Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
This patch alone does not make sense, and I guess it breaks
compilation on Windows if we stop at it during a bisect (because it
adds two unused static symbols).
It can be squashed in patch 3 where thread_log_last_error is first used.
On Mon, Apr 25, 2022 at 10:25:30AM +0200, David Marchand wrote:
> On Tue, Apr 12, 2022 at 12:44 PM Tyler Retzlaff
> <roretzla@linux.microsoft.com> wrote:
> >
> > 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 <navasile@microsoft.com>
> > Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
>
> This patch alone does not make sense, and I guess it breaks
> compilation on Windows if we stop at it during a bisect (because it
> adds two unused static symbols).
interesting, didn't happen locally but i can see that it should have.
> It can be squashed in patch 3 where thread_log_last_error is first used.
yep, makes sense to me. i'll push a new version.
thanks
@@ -1,5 +1,6 @@
/* SPDX-License-Identifier: BSD-3-Clause
* Copyright 2021 Mellanox Technologies, Ltd
+ * Copyright (C) 2022 Microsoft Corporation
*/
#include <rte_common.h>
@@ -11,6 +12,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)
+{
+ 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;
+}
+
+static int
+thread_log_last_error(const char *message)
+{
+ DWORD error = GetLastError();
+ RTE_LOG(DEBUG, EAL, "GetLastError()=%lu: %s\n", error, message);
+
+ return thread_translate_win32_error(error);
+}
+
int
rte_thread_key_create(rte_thread_key *key,
__rte_unused void (*destructor)(void *))