On Wed, Jun 09, 2021 at 02:03:48AM +0300, Dmitry Kozlyuk wrote:
> 2021-06-04 16:44 (UTC-0700), Narcisa Ana Maria Vasile:
> > From: Narcisa Vasile <navas...@microsoft.com>
> >
> > Use a portable, type-safe representation for the thread identifier.
> > Add functions for comparing thread ids and obtaining the thread id
> > for the current thread.
> >
> > Signed-off-by: Narcisa Vasile <navas...@microsoft.com>
> > ---
> > lib/eal/common/rte_thread.c | 105 ++++++++++++++++++
> > lib/eal/include/rte_thread.h | 53 +++++++--
> > lib/eal/include/rte_thread_types.h | 10 ++
> > .../include/rte_windows_thread_types.h | 10 ++
> > lib/eal/windows/rte_thread.c | 17 +++
> > 5 files changed, 186 insertions(+), 9 deletions(-)
> > create mode 100644 lib/eal/common/rte_thread.c
> > create mode 100644 lib/eal/include/rte_thread_types.h
> > create mode 100644 lib/eal/windows/include/rte_windows_thread_types.h
>
> It is strange that new files are being filled in the series, but are neither
> compiled nor installed until the last patch. Any reason not to replace
> lib/eal/unix/rte_thread.c starting from this patch?
>
Replaced unix/rte_thread.c starting from here.
> A better name for rte_thread_types.h would be rte_posix_thread_types.h
> to indicate this file is not really common.
>
> >
> > +rte_thread_t
> > +rte_thread_self(void)
> > +{
> > + rte_thread_t thread_id = { 0 };
>
> (Applies to entire series.)
> Please do not initialize variables when you intend to overwrite them.
> This prevents compiler from reporting code paths where the variable is used
> before a proper assignment.
Thanks for the explanation, Dmitry!
>
>
> [...]
> > diff --git a/lib/eal/include/rte_thread.h b/lib/eal/include/rte_thread.h
> > index 8be8ed8f36..347df1a6ae 100644
> directly, see lib/eal/$arch/rte_atomic_64.h for examples.
>
> Note that rte_*_thread_types.h should be `indirect_headers`, not `headers`
> in `meson.build` so that they're not checked by `buildtools/chkincs` when is
> gets enabled for Windows. This is especially
> true for lib/eal/common/rte_thread.h that cannot work without pthread.
>
> However, in later patches these headers only contain mutex bits,
> see the comment to pathc 07/10 about them. Maybe we don't need these files
> after all.
Agreed, I was able to remove them after reimplementing the mutex functions.
