Re: [dpdk-dev] [PATCH v5 03/10] windows/eal: translate Windows errors to errno-style errors

[Narcisa Ana Maria Vasile <navasile@linux.microsoft.com>]

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