2011/8/11 Colin Guthrie <[email protected]>: > 'Twas brillig, and Lu Guanqun at 11/08/11 10:54 did gyre and gimble: >> On Thu, Aug 11, 2011 at 04:30:12PM +0800, Colin Guthrie wrote: >>> 'Twas brillig, and Lu Guanqun at 11/08/11 02:59 did gyre and gimble: >>>> According to the principle of DRY (don't repeat yourself), remove the code >>>> for >>>> setting thread name in thread-posix.c. >>>> >>>> Signed-off-by: Lu Guanqun <[email protected]> >>>> --- >>>> src/pulsecore/thread-posix.c | 20 ++++++++++---------- >>>> 1 files changed, 10 insertions(+), 10 deletions(-) >>>> >>>> diff --git a/src/pulsecore/thread-posix.c b/src/pulsecore/thread-posix.c >>>> index 3f4ae5c..9a8c51b 100644 >>>> --- a/src/pulsecore/thread-posix.c >>>> +++ b/src/pulsecore/thread-posix.c >>>> @@ -65,15 +65,19 @@ static void thread_free_cb(void *p) { >>>> >>>> PA_STATIC_TLS_DECLARE(current_thread, thread_free_cb); >>>> >>>> +static void set_thread_name(const char *name) { >>>> +#ifdef __linux__ >>>> + prctl(PR_SET_NAME, name); >>>> +#elif defined(HAVE_PTHREAD_SETNAME_NP) && defined(OS_IS_DARWIN) >>>> + pthread_setname_np(name); >>>> +#endif >>>> +} >>>> + >>>> static void* internal_thread_func(void *userdata) { >>>> pa_thread *t = userdata; >>>> pa_assert(t); >>>> >>>> -#ifdef __linux__ >>>> - prctl(PR_SET_NAME, t->name); >>>> -#elif defined(HAVE_PTHREAD_SETNAME_NP) && defined(OS_IS_DARWIN) >>>> - pthread_setname_np(t->name); >>>> -#endif >>>> + set_thread_name(t->name); >>>> >>>> t->id = pthread_self(); >>>> >>>> @@ -175,11 +179,7 @@ void pa_thread_set_name(pa_thread *t, const char >>>> *name) { >>>> pa_xfree(t->name); >>>> t->name = pa_xstrdup(name); >>>> >>>> -#ifdef __linux__ >>>> - prctl(PR_SET_NAME, name); >>>> -#elif defined(HAVE_PTHREAD_SETNAME_NP) && defined(OS_IS_DARWIN) >>>> - pthread_setname_np(name); >>>> -#endif >>>> + set_thread_name(name); >>>> } >>>> >>>> const char *pa_thread_get_name(pa_thread *t) { >>> >>> >>> Am I blind or is pa_thread_set_name() itself redundant? I cannot find >>> any calls to it.... >> >> Good catch! I overlooked the code, I thought it was called in >> pa_thread_new(). Do we need to add it in pa_thread_new() btw?
Well, it looks like it is being set in internal_thread_func, so no need to first set it in pa_thread_new. Also it looks like the function that is used [prctl(PR_SET_NAME, name)] should be called from the running thread itself, so internal_thread_func instead of pa_thread_new is obviously the place to do it. That leads me to suspect the whole code in pa_thread_set_name. That seems to alter the name of the thread invoking the function, instead of the thread pointed to by the first argument. Maarten > I have no idea..., but it does seem like it's missing... > > Also there is a memory leak in the case when pthread_create() fails > (t->name is not freed). > > > Anyone have any specific info here? > > Col > > > > > > -- > > Colin Guthrie > gmane(at)colin.guthr.ie > http://colin.guthr.ie/ > > Day Job: > Tribalogic Limited [http://www.tribalogic.net/] > Open Source: > Mageia Contributor [http://www.mageia.org/] > PulseAudio Hacker [http://www.pulseaudio.org/] > Trac Hacker [http://trac.edgewall.org/] > > _______________________________________________ > pulseaudio-discuss mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss > _______________________________________________ pulseaudio-discuss mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
