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 <navas...@microsoft.com>
> >Signed-off-by: Tyler Retzlaff <roret...@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.
