[v5,03/10] windows/eal: translate Windows errors to errno-style errors
Checks
Commit Message
From: Narcisa Vasile <navasile@microsoft.com>
Add function to translate Windows error codes to
errno-style error codes.
Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
---
lib/librte_eal/windows/rte_thread.c | 65 ++++++++++++++++++++++-------
1 file changed, 50 insertions(+), 15 deletions(-)
Comments
> Subject: [PATCH v5 03/10] windows/eal: translate Windows errors to errno-
> style errors
>
> External email: Use caution opening links or attachments
>
>
> From: Narcisa Vasile <navasile@microsoft.com>
>
> Add function to translate Windows error codes to errno-style error codes.
>
> Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
> ---
> lib/librte_eal/windows/rte_thread.c | 65 ++++++++++++++++++++++-------
> 1 file changed, 50 insertions(+), 15 deletions(-)
>
> diff --git a/lib/librte_eal/windows/rte_thread.c
> b/lib/librte_eal/windows/rte_thread.c
> index b29336cbd..e9181b47f 100644
> --- a/lib/librte_eal/windows/rte_thread.c
> +++ b/lib/librte_eal/windows/rte_thread.c
> @@ -12,6 +12,47 @@ struct eal_tls_key {
> DWORD thread_index;
> };
>
> +/* Translates the most common error codes related to threads */ static
> +int rte_thread_translate_win32_error(DWORD error) {
This DWORD error will always the output of GetLastError()? If so can we move it inside the function?
Also, I don't think this is a thread specific function, other implementations can use it in the future, maybe move it to rte_windows.h?
> + 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;
> + }
> +
> + return EINVAL;
> +}
> +
> rte_thread_t
> rte_thread_self(void)
> {
> @@ -87,15 +128,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(GetLastError());
> }
> return 0;
> }
> @@ -103,16 +142,14 @@ rte_thread_key_create(rte_thread_key *key, int
> rte_thread_key_delete(rte_thread_key key) {
> - if (!key) {
> + if (key == NULL) {
> RTE_LOG(DEBUG, EAL, "Invalid TLS key.\n");
> - rte_errno = EINVAL;
> - return -1;
> + return EINVAL;
> }
> if (!TlsFree(key->thread_index)) {
> RTE_LOG_WIN32_ERR("TlsFree()");
> free(key);
> - rte_errno = ENOEXEC;
> - return -1;
> + return rte_thread_translate_win32_error(GetLastError());
> }
> free(key);
> return 0;
> @@ -123,17 +160,15 @@ rte_thread_value_set(rte_thread_key key, const
> void *value) {
> char *p;
>
> - if (!key) {
> + if (key == NULL) {
> RTE_LOG(DEBUG, EAL, "Invalid TLS key.\n");
> - rte_errno = EINVAL;
> - return -1;
> + return EINVAL;
> }
> /* discard const qualifier */
> p = (char *) (uintptr_t) value;
> if (!TlsSetValue(key->thread_index, p)) {
> RTE_LOG_WIN32_ERR("TlsSetValue()");
> - rte_errno = ENOEXEC;
> - return -1;
> + return rte_thread_translate_win32_error(GetLastError());
> }
> return 0;
> }
> @@ -143,7 +178,7 @@ rte_thread_value_get(rte_thread_key key) {
> void *output;
>
This function is missing the change to rte_thread_translate_win32_error.
Aldo need to change function docu.
> - if (!key) {
> + if (key == NULL) {
> RTE_LOG(DEBUG, EAL, "Invalid TLS key.\n");
> rte_errno = EINVAL;
> return NULL;
> --
> 2.30.0.vfs.0.2
> Also, I don't think this is a thread specific function, other implementations can use it in the future, maybe move it to rte_windows.h?
I'd suggest that it's probably better suited to a .c file rather
than a header. As an example of what it might end up like
see https://github.com/wpdk/wpdk/blob/master/src/error.c
Regards,
Nick
> Subject: Re: [dpdk-dev] [PATCH v5 03/10] windows/eal: translate Windows
> errors to errno-style errors
>
> External email: Use caution opening links or attachments
>
>
> > Also, I don't think this is a thread specific function, other implementations
> can use it in the future, maybe move it to rte_windows.h?
>
> I'd suggest that it's probably better suited to a .c file rather than a header. As
> an example of what it might end up like see
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> ub.com%2Fwpdk%2Fwpdk%2Fblob%2Fmaster%2Fsrc%2Ferror.c&data=
> 04%7C01%7Ctalshn%40nvidia.com%7C4ab2e17b824842b9f83208d8f48ab4ec
> %7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637528223860057761
> %7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiL
> CJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=AnBHMtSobODNzb
> Uhun3GVnsTiePl%2BQzfPSASlur81Ks%3D&reserved=0
>
> Regards,
> Nick
>
Agreed, thanks Nick.
On Wed, Mar 31, 2021 at 01:56:09PM +0000, Tal Shnaiderman wrote:
> > Subject: [PATCH v5 03/10] windows/eal: translate Windows errors to errno-
> > style errors
> >
> > External email: Use caution opening links or attachments
> >
> >
> > From: Narcisa Vasile <navasile@microsoft.com>
> >
> > Add function to translate Windows error codes to errno-style error codes.
> >
> > Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
> > ---
> > lib/librte_eal/windows/rte_thread.c | 65 ++++++++++++++++++++++-------
> > 1 file changed, 50 insertions(+), 15 deletions(-)
> >
> > diff --git a/lib/librte_eal/windows/rte_thread.c
> > b/lib/librte_eal/windows/rte_thread.c
> > index b29336cbd..e9181b47f 100644
> > --- a/lib/librte_eal/windows/rte_thread.c
> > +++ b/lib/librte_eal/windows/rte_thread.c
> > @@ -12,6 +12,47 @@ struct eal_tls_key {
> > DWORD thread_index;
> > };
> >
> > +/* Translates the most common error codes related to threads */ static
> > +int rte_thread_translate_win32_error(DWORD error) {
>
> This DWORD error will always the output of GetLastError()? If so can we move it inside the function?
Yes, I think we can move the call to GetLastError() inside the function, thanks Tal.
>
> Also, I don't think this is a thread specific function, other implementations can use it in the future, maybe move it to rte_windows.h?
I think it's better to keep these error-translation functions inside a specific module (threads, memory, etc.).
The reason for that is that the same error code may mean different things in different modules. When I implemented
this function I've went through all the Windows threads functions and noted down the type of errors that GetLastError()
returns for them and what they mean. For a different module, a different set of errors might be more suitable,
so to keep the translations as semantically compatible as possible to Linux, it's probably better to keep it
at the module level (threading in this case).
>
> > + 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;
> > + }
> > +
> > + return EINVAL;
> > +}
> > +
> > @@ -143,7 +178,7 @@ rte_thread_value_get(rte_thread_key key) {
> > void *output;
> >
>
> This function is missing the change to rte_thread_translate_win32_error.
> Aldo need to change function docu.
Thanks, will fix and send v6.
>
> > - if (!key) {
> > + if (key == NULL) {
> > RTE_LOG(DEBUG, EAL, "Invalid TLS key.\n");
> > rte_errno = EINVAL;
> > return NULL;
> > --
> > 2.30.0.vfs.0.2
@@ -12,6 +12,47 @@ struct eal_tls_key {
DWORD thread_index;
};
+/* Translates the most common error codes related to threads */
+static int rte_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;
+
+ default:
+ return EINVAL;
+ }
+
+ return EINVAL;
+}
+
rte_thread_t
rte_thread_self(void)
{
@@ -87,15 +128,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(GetLastError());
}
return 0;
}
@@ -103,16 +142,14 @@ rte_thread_key_create(rte_thread_key *key,
int
rte_thread_key_delete(rte_thread_key key)
{
- if (!key) {
+ if (key == NULL) {
RTE_LOG(DEBUG, EAL, "Invalid TLS key.\n");
- rte_errno = EINVAL;
- return -1;
+ return EINVAL;
}
if (!TlsFree(key->thread_index)) {
RTE_LOG_WIN32_ERR("TlsFree()");
free(key);
- rte_errno = ENOEXEC;
- return -1;
+ return rte_thread_translate_win32_error(GetLastError());
}
free(key);
return 0;
@@ -123,17 +160,15 @@ rte_thread_value_set(rte_thread_key key, const void *value)
{
char *p;
- if (!key) {
+ if (key == NULL) {
RTE_LOG(DEBUG, EAL, "Invalid TLS key.\n");
- rte_errno = EINVAL;
- return -1;
+ return EINVAL;
}
/* discard const qualifier */
p = (char *) (uintptr_t) value;
if (!TlsSetValue(key->thread_index, p)) {
RTE_LOG_WIN32_ERR("TlsSetValue()");
- rte_errno = ENOEXEC;
- return -1;
+ return rte_thread_translate_win32_error(GetLastError());
}
return 0;
}
@@ -143,7 +178,7 @@ rte_thread_value_get(rte_thread_key key)
{
void *output;
- if (!key) {
+ if (key == NULL) {
RTE_LOG(DEBUG, EAL, "Invalid TLS key.\n");
rte_errno = EINVAL;
return NULL;