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

[Narcisa Ana Maria Vasile]

On Thu, Apr 29, 2021 at 03:50:38AM +0300, Dmitry Kozlyuk wrote:
> 2021-04-02 18:39 (UTC-0700), Narcisa Ana Maria Vasile:
> > From: Narcisa Vasile <navas...@microsoft.com>
> > 
> > Add function to translate Windows error codes to
> > errno-style error codes.
> > 
> > Signed-off-by: Narcisa Vasile <navas...@microsoft.com>
> 
> Commit topic should be "eal/windows", not "windows/eal".
> 
> > ---
> >  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
> 
> > @@ -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.
> 

  Thanks Dmitry! I can split the translation function into something like:
  static int rte_thread_translate_win32_error(DWORD error) {
        switch(error) {
            [...]
        }
  } 

  static int get_error_and_translate(void) {
     return rte_thread_translate_win32_error(GetLastError());
  }

  Is the above what you meant?

  Here, however, I don't think the wrapper over GetLastError() will be of much 
help, as 
  we still need to do something like:
  if ((*key)->thread_index == TLS_OUT_OF_INDEXES) {
       ret = GetLastError();
       RTE_LOG(DEBUG, EAL, "TlsAlloc() failed, GetLastError()=%lu: ", ret);
       free(*key);
       return rte_thread_translate_win32_error(ret);
  }

> Same applies below in this file.
> 
> [...]