Re: [PATCH] Linux: Implement per-thread file system attributes

[Florian Weimer <fweimer at redhat dot com>]

* Carlos O'Donell:

> The thread_perthreadfs_indirect uses the NULL attribute,
> and not PTHREAD_PER_PROCESS_NP explicitly, and it's an
> implementation detail that this happens to be the same 
> code path e.g. we copy the default global attr, then adjust
> the scope flags. This might change in the future and it would
> be nice to catch any breakage in a distinct test for this.
>
> The test would be a basic flag test looking to validate the
> stickyness in a simpler test.
>
> The simpler test should have the parent threads permute over
> [null, per-process, per-thread], and the child iterate over
> [null, per-process, per-thread], and validate the expected
> results.

Is it sufficient to verify that the thread attribute is set correctly,
or do you want a fully functional test?

If the latter, I will split the indirect flag into direct, indirect with
default attribute, and indirect with non-default attribute because I
would have to replicate a lot of the other test functionality to verify
per-thread separation or the lack thereof.

>> +/* Return true if the thread has per-thread file system
>> +   attributes.  */
>> +static bool
>> +perthread_flag (pthread_t thr)
>> +{
>> +  pthread_attr_t attr;
>> +  int ret = pthread_getattr_np (thr, &attr);
>> +  if (ret != 0)
>> +    {
>> +      errno = ret;
>> +      FAIL_EXIT1 ("pthread_getattr_np: %m");
>> +    }
>> +  int flag = -1;
>> +  pthread_attr_getperthreadfs_np (&attr, &flag);
>> +  if (flag != 0)
>> +    TEST_COMPARE (flag, 1);
>
> Magic number.
>
> Please use some intermediate define to call this out.
>
> #define CHECK_PTHREAD_PER_THREAD_FS 1
>
> ... and use in place of 1.
>
> This way it's grep-able when looking for references to
> the enumerated value.

Right, I realized that as well.  I've now got:

/* Return true if the thread has per-thread file system
   attributes.  */
static bool
perthread_flag (pthread_t thr)
{
  pthread_attr_t attr;
  int ret = pthread_getattr_np (thr, &attr);
  if (ret != 0)
    {
      errno = ret;
      FAIL_EXIT1 ("pthread_getattr_np: %m");
    }
  int flag = -1;
  pthread_attr_getperthreadfs_np (&attr, &flag);
  if (flag != PTHREAD_PER_THREAD_NP)
    TEST_COMPARE (flag, PTHREAD_PER_PROCESS_NP);
  xpthread_attr_destroy (&attr);
  return flag == PTHREAD_PER_THREAD_NP;
}

Thanks,
Florian