Re: [Xenomai-core] Re: [syscall.c] rt_bind_queue/heap()

[Philippe Gerum <rpm@xenomai.org>]

Philippe Gerum wrote:
> Dmitry Adamushko wrote:
> 
>> On 11/10/05, Philippe Gerum <rpm@xenomai.org> wrote:
>>
>>>> ...
>>>> So,
>>>>
>>>> 1) don't display such names in /proc;
>>>> 2) make a common mechanism for both spaces.
>>>>
>>>> rt_mutex_create()   // for other objects as well
>>>> {
>>>> ...
>>>> - xnobject_copy_name(mutex->name,name);
>>>> +xnobject_create_name(mutex->name,name, object);
>>>> ...
>>>> }
>>>>
>>>> xnobject_create_name(dst,src, object)
>>>> {
>>>>      if (src)
>>>>            xnobject_copy_name(mutex->name,name);
>>>>      else
>>>>            snprintf(dst, XNOBJECT_NAME_LEN, "%p", object);
>>>> }
>>>>
>>>> // the slightly ugly thing is that we need to be sure that dst is
>>>> always object->name so it's really of XNOBJECT_NAME_LEN size.
>>>>
>>>
>>> We should pass the size along with the copy buffer, mainly for 
>>> readability, and
>>> also as a safety belt.
>>
>>
>>
>> We don't need that at all, since now we don't want to change a
>> behaviour of NULL-named objects created from kernel-space.
>>
>>
>>
>>>> Frankly, I like this approach better. All objects get some name if
>>>> they are NULL-named by a user and the code changes are a bit more
>>>> graceful. To show them in /proc or not is still another question.
>>>
>>>
>>> I'd like we don't generalize an exception case here. The fact that 
>>> for some
>>> internal reason we need to put a dummy name on anon objects created from
>>> user-space should not make all anon objects from kernel space have a 
>>> registry
>>> slot too, this would just be overkill (registry slots don't come for 
>>> free actually).
>>>
>>> That said, the remaining point basically is: how do we avoid anon 
>>> objects with
>>> auto-generated names from appearing under /proc? I'm not a fan of 
>>> "magic values"
>>> which would distinguish a real name from a dummy one, so I'd rather 
>>> use the
>>> existing possibility of passing a NULL pnode pointer to 
>>> rt_registry_enter(),
>>> which would still index an object without actually exporting it to 
>>> the /proc
>>> interface. The question is: how could we make the *_create() calls 
>>> invoking
>>> rt_registry_enter() with a NULL pnode when applicable, without 
>>> changing the
>>> kernel-based interface of the API.
>>
>>
>>
>> Well, I guess the only way a given *_create() call may determine a
>> certain environment (kernel mode vs. user mode) is by analyzing some
>> of the given arguments (ok, in case of the user mode, a previous call
>> on the stack has been one of the syscall.c:__rt_object_create() ones,
>> so we may track the stack... ok-ok, I'm just kidding :)
>> There are only 2 arguments: rt_object *object or char *name.
>> We probably can't rely on the first one since it may contain anything
>> and we don't want to force a user to set it up to something certain:
>>
>> /* kernel mode */
>> RT_MUTEX m;
>> bzero(&m); // because we use some RT_MUTEX's internal fields when
>> making a call from syscall.c:__rt_mutex_create(), e.g. m->handle =
>> DONT_USE_REGISTRY;
>>
>> rt_mutex_create(&m, NULL);
>>
>> So the only way is to deal somehow with the "name" argument. There is
>> a difference already.
>>
>> rt_mutex_create() being called from:
>>
>> 1) kernel mode:  name == NULL;
>> 2) user mode:    name == "\0" (see, syscall.c:__rt_mutex_create(), so
>> *name == NULL;
>>
>> Then a fix would look like:
>>
>> rt_object_create()
>> {
>> ...
>>
>> - if (name && *name)
>> + if (name)
>>         {
>> + if (*name)
>>    err = 
>> rt_registry_enter(mutex->name,mutex,&mutex->handle,&__mutex_pnode);
>> + else {
>> + // this is a NULL-named object being created from the user-mode or
>> + // "\0"-named object from the kernel-mode
>> +xnobject_create_name(mutex->name, OBJNAME_MAX_SIZE, mutex);
>> + err = rt_registry_enter(mutex->name,mutex,&mutex->handle,NULL);
>> +}
>>
>>         if (err)
>>             rt_mutex_delete(mutex);
>>         }
>> }
>>
>> Maybe a bit ugly but should work. eerrr... I can't come up with a
>> better solution at the moment.
>>
> 
> At least the magic value here would make some sense, NULL=unregistered, 
> empty=registered,not-exported,

Actually, this is "registered,anonymous,hence not-exported"

  otherwise=registered,exported. Looks ok
> for that purpose, since we do need the additional 2nd state here.
> 

-- 

Philippe.