Re: [PATCH] Add test for pthread_setname_np and pthread_getname_np.

["Carlos O'Donell" <carlos at redhat dot com>]

On 12/16/2013 08:35 AM, Adhemerval Zanella wrote:
> On 12-12-2013 04:29, Carlos O'Donell wrote:
>> On 12/12/2013 12:50 AM, Carlos O'Donell wrote:
>>> On 12/10/2013 10:27 AM, Torvald Riegel wrote:
>>>> On Tue, 2013-12-10 at 01:07 -0500, Carlos O'Donell wrote:
>>>>> I've had this kicking around since Ondrej added the manual
>>>>> entry for these functions and we all agreed that they should
>>>>> do nothing with NULL input.
>>>>>
>>>>> This adds 4 test for pthread_setname_np and pthread_getname_np.
>>>>>
>>>>> Test 1: Get the name of the task via pthread_getname_np and /proc
>>>>>         and verify that they both match.
>>>>>
>>>>> Test 2: Test setting the name and then independently verify it
>>>>>         was set via /proc.
>>>>>
>>>>> Test 3: Test setting a name that is one-byte too big and fails
>>>>>         with the correct error return *and* that the process
>>>>>         name is not modified.
>>>>>
>>>>> Test 4: Verify that setting the longest name works.
>>>>>
>>>>> Tested on x86-64 with no regressions.
>>>>>
>>>>> [carlos@koi glibc]$ cat nptl/tst-setgetname.out 
>>>>> PASS: Test 1 - pthread_getname_np and /proc agree.
>>>>> PASS: Test 2 - Value used in pthread_setname_np and /proc agree.
>>>>> PASS: Test 3 - pthread_setname_np returned ERANGE for a process name that was too long.
>>>>> PASS: Test 3 - Original name unchanged after pthread_setname_np returned ERANGE.
>>>>> PASS: Test 4 - Longest name set via pthread_setname_np agrees with /proc.
>>>>>
>>>>> OK to checkin?
>>>> Looks good to me.
>>> Thanks, I've checked this in.
>>>
>>> Cheers,
>>> Carlos.
>>>
>> Final version with two more tests is what I checked in:
>>
>> * Check that calling pthread_getname_np with small buffer results in ERANGE
>>   and buffer remains unmodified.
>>
>> * Check that you can read back the longest name correctly.
>>
>> 2013-12-09  Carlos O'Donell  <carlos@redhat.com>
>>
>> 	* sysdeps/unix/sysv/linux/tst-setgetname.c: New file.
>> 	* sysdeps/unix/sysv/linux/Makefile (tests): Add tst-setgetname.
>>
>>
> Hi Carlos,
> 
> The /proc/pid/tasks/tid/comm was added in linux 2.6.33 (commit 4614a696bd1c3a9af3a08f0e5874830a85b889d4) 
> and current tests are failing in order kernels. Should we keep it failing or add check for if errno
> is ENOENT (for get_self_comm return) to skip the tests?
> 

We should fix this for 2.19.

We should add ENOENT and skip the tests.

I'll check this in unless you do it first :-)

diff --git a/nptl/sysdeps/unix/sysv/linux/tst-setgetname.c b/nptl/sysdeps/unix/sysv/linux/tst-setgetname.c
index 2aceba5..0235d37 100644
--- a/nptl/sysdeps/unix/sysv/linux/tst-setgetname.c
+++ b/nptl/sysdeps/unix/sysv/linux/tst-setgetname.c
@@ -99,6 +99,17 @@ do_test (int argc, char **argv)
   if (res == 0)
     {
       res = get_self_comm (gettid (), name_check, TASK_COMM_LEN);
+
+      /* On this first test we look for ENOENT to be returned from
+         get_self_comm to indicate that the kernel is older than
+         2.6.33 and doesn't contain comm within the proc structure.
+         In that case we skip the entire test.  */
+      if (res == ENOENT)
+       {
+         printf ("SKIP: The kernel does not have /proc/self/task/%%lu/comm.\n");
+         return 0;
+       }
+
       if (res == 0)
        {
          if (strncmp (name, name_check, strlen (BIG_NAME)) == 0)
---

Cheers,
Carlos.