Re: [PATCH v3 07/20] util: add API to fetch the current thread name

[Markus Armbruster <armbru@redhat.com>]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Fri, Sep 19, 2025 at 10:59:18AM +0200, Markus Armbruster wrote:
>> Daniel P. Berrangé <berrange@redhat.com> writes:
>> 
>> > This will be used to include the thread name in error reports
>> > in a later patch. It returns a const string stored in a thread
>> > local to avoid memory allocation when it is called repeatedly
>> > in a single thread. This makes the assumption that the thread
>> > name is set at the very start of the thread, which is the case
>> > when using qemu_thread_create.
>> 
>> What happens when the assumption is violated?
>
> You will get an operating system default thread name,
> which on Linux will default to the unqualified binary
> name based on argv[0]

Suggest something like "The thread name should be set at the very start
of the thread, which is the case when using qemu_thread_create."

>> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>> > ---
>> >  include/qemu/thread.h    |  1 +
>> >  meson.build              | 21 +++++++++++++++++
>> >  util/qemu-thread-posix.c | 28 ++++++++++++++++++++++-
>> >  util/qemu-thread-win32.c | 49 ++++++++++++++++++++++++++++++++++++----
>> >  4 files changed, 94 insertions(+), 5 deletions(-)
>
>
>> > diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
>> > index 275445ed94..fbb94ca97b 100644
>> > --- a/util/qemu-thread-posix.c
>> > +++ b/util/qemu-thread-posix.c
>> > @@ -18,7 +18,7 @@
>> >  #include "qemu/tsan.h"
>> >  #include "qemu/bitmap.h"
>> >  
>> > -#ifdef CONFIG_PTHREAD_SET_NAME_NP
>> > +#if defined(CONFIG_PTHREAD_SET_NAME_NP) || defined(CONFIG_PTHREAD_GET_NAME_NP)
>> >  #include <pthread_np.h>
>> >  #endif
>> >  
>> > @@ -532,3 +532,29 @@ void *qemu_thread_join(QemuThread *thread)
>> >      }
>> >      return ret;
>> >  }
>> > +
>> > +#ifndef PTHREAD_MAX_NAMELEN_NP
>> > +#define PTHREAD_MAX_NAMELEN_NP 16
>> 
>> Feels a bit tight.  32?
>
> On Linux this constant is not defined, but the manpage says
>
>   The thread name is a meaningful C language string, whose length is
>   restricted to 16 characters, including the terminating null byte ('\0')
>
> so I defined the constant to match the non-Linux constant
> name, but with the Linux documented max length.

I see.

Maybe mention in a comment or the commit message?

[...]