Re: [PATCH] Linux: Implement per-thread user and group IDs

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

On 7/1/19 12:49 AM, Florian Weimer wrote:
> * Carlos O'Donell:
> 
>>> We might have to use SIGSETXID for something else in the future (or
>>> perhaps we already do).  I would leave this as it is today.
>>
>> Can you expand on this?
>>
>> Today SIGSETXID is reserved explicitly for this use, and it's a per-process
>> signal handler, so it's always active until you remove the last use of
>> SIGSETXID by replacing it with a kernel mechanism for transitioning IDs.
> 
> I meant that we might want to use the signal broadcast for something else,
> not just set*id calls.

We can remove the blocking code / assertions at that point?

I think even a minimal check in the handler that checks to see if there
is a per-thread uid/gid in effect and does nothing might be sufficient
here to pevent any accidental usage?

Right now your only consistency checks are on the caller side.

>>>>> +* Likewise, on Linux, threads can now be created in such a way that they
>>>>
>>>> s/Likewise, on/On/g
>>>
>>> There's a preceding NEWS entry with similar wording, so I thought the
>>> “Likewise” would make sense here.
>>
>> Each news entry should stand on its own so it can be quoted, or so it makes
>> sense when reading just one entry.
> 
> Okay, I'll change this.
> 
>>>>> +/* Set *SCOPE to PTHREAD_PER_PROCESS_NP or PTHREAD_PER_THREAD_NP,
>>>>> +   depending on the state of *ATTR.  */
>>>>
>>>> Wrong comment.
>>>>
>>>>> +int pthread_attr_getperthreadids_np (const pthread_attr_t
>>>>> *__restrict __attr,
>>>>> +				     int *__restrict __scope)
>>>>> +  __THROW __nonnull ((1, 2));
>>>>> +
>>>
>>> Sorry, wrong comment in what way?
>>
>> It says "Set ..." but this is the getter.
> 
> *SCOPE is an out parameter, so “Set” is correct?  I can use “Copy … to
>  …” instead if that's clearer.

Ah, I see what you were intending.

With setter/getter APIs it's wise to use the language of "set" and "get"
relative to the object being manipulated.

e.g.

/* Get the thread scope from *ATTR and store in *SCOPE.  */

vs.

/* Set the thread scope in *ATTR from the value *SCOPE.  */

-- 
Cheers,
Carlos.