[pulseaudio-discuss] [PATCH v2] thread-posix: remove duplicate code for setting thread name

[Colin Guthrie]

'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 <guanqun.lu at intel.com>
>>> ---
>>>  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?


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/]