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

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

[Dmitry Adamushko]

[-- Attachment #1: Type: text/plain, Size: 1623 bytes --]


Philippe Gerum <rpm@xenomai.org> wrote on 10.10.2005 13:09:13:

> Dmitry Adamushko wrote:
> > Philippe Gerum <rpm@xenomai.org> wrote on 05.10.2005 14:13:07:
> >
> >  >
> >  > Your patch is ok, my implementation was uselessly convoluted.
> >
> > Ok, then it's enclosed.
> >
> > 2005-10-05  Dmitry Adamushko  <dmitry.adamushko@domain.hid>
>
> Applied, thanks.
> ...

btw,

It looks like something wrong with the xenomai mailing list. I have got
your answer but only one copy (you specified my own e-mail) and I haven't
got my own message on the list (but I've cc'ed "xenomai-core").

One more thing. I had a discussion with Steven Seeger regarding the use of
NULL-named objects from the user space. I cc'ed you but probably you were
too buzy at that time. The problem is that one may create successfully a
NULL-named object but then there is no way to use it since all further
calls give an error (some funny stuff there indeed :)

So a simple fix would look like:

for all rt_OBJECT_create() calls in libnative:

int rt_mutex_create(RT_MUTEX *mutex, const char *name)
{

+     if (!name)
+           return -E_SOMETHING; // E_INVAL?

      return XENOMAI_SKINCALL2(__xeno_muxid,
                               __xeno_mutex_create,
                               mutex,
                               name);
}

What do you think? Or are there any reasons to keep it as is now?


p.s. I have cc'ed "xenomai@xenomai.org" for both reasons to test the
mailing list and, well, maybe someone else has an opinion on the matter.


> --
>
> Philippe.

---
Best regards,
Dmitry

[-- Attachment #2: Type: text/html, Size: 2091 bytes --]

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

[Philippe Gerum]

Dmitry Adamushko wrote:
> Philippe Gerum <rpm@xenomai.org> wrote on 10.10.2005 13:09:13:
> 
>  > Dmitry Adamushko wrote:
>  > > Philippe Gerum <rpm@xenomai.org> wrote on 05.10.2005 14:13:07:
>  > >
>  > >  >
>  > >  > Your patch is ok, my implementation was uselessly convoluted.
>  > >
>  > > Ok, then it's enclosed.
>  > >
>  > > 2005-10-05  Dmitry Adamushko  <dmitry.adamushko@domain.hid>
>  >
>  > Applied, thanks.
>  > ...
> 
> btw,
> 
> It looks like something wrong with the xenomai mailing list. I have got 
> your answer but only one copy (you specified my own e-mail) and I 
> haven't got my own message on the list (but I've cc'ed "xenomai-core").
>

Your mail to the list was blocked since you used an unregistered address to send 
it (datacon.at).

> One more thing. I had a discussion with Steven Seeger regarding the use 
> of NULL-named objects from the user space. I cc'ed you but probably you 
> were too buzy at that time. The problem is that one may create 
> successfully a NULL-named object but then there is no way to use it 
> since all further calls give an error (some funny stuff there indeed :)
> 
> So a simple fix would look like:
> 
> for all rt_OBJECT_create() calls in libnative:
> 
> int rt_mutex_create(RT_MUTEX *mutex, const char *name)
> {
> 
> + if (!name)
> + return -E_SOMETHING; // E_INVAL?
> 
> return XENOMAI_SKINCALL2(__xeno_muxid,
> __xeno_mutex_create,
> mutex,
> name);
> }
> 
> What do you think? Or are there any reasons to keep it as is now?
> 

The reason is to allow anonymous objects to be created, so that the descriptor 
could only be shared by tasks belonging to the same address space if they happen 
to all have access to the descriptor's memory. Kind of semi-private object if 
you want. The fact that such an object is non-bindable should not make it 
unusable actually.

> 
> p.s. I have cc'ed "xenomai@xenomai.org" for both reasons to test the 
> mailing list and, well, maybe someone else has an opinion on the matter.
> 
> 
>  > --
>  >
>  > Philippe.
> 
> ---
> Best regards,
> Dmitry
> 


-- 

Philippe.

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

[Dmitry Adamushko]

> > One more thing. I had a discussion with Steven Seeger regarding the use
> > of NULL-named objects from the user space. I cc'ed you but probably you
> > were too buzy at that time. The problem is that one may create
> > successfully a NULL-named object but then there is no way to use it
> > since all further calls give an error (some funny stuff there indeed :)
> >
> > So a simple fix would look like:
> >
> > for all rt_OBJECT_create() calls in libnative:
> >
> > int rt_mutex_create(RT_MUTEX *mutex, const char *name)
> > {
> >
> > + if (!name)
> > + return -E_SOMETHING; // E_INVAL?
> >
> > return XENOMAI_SKINCALL2(__xeno_muxid,
> > __xeno_mutex_create,
> > mutex,
> > name);
> > }
> >
> > What do you think? Or are there any reasons to keep it as is now?
> >

> The reason is to allow anonymous objects to be created, so that the descriptor
> could only be shared by tasks belonging to the same address space if they
> happen
> to all have access to the descriptor's memory. Kind of semi-private object if
> you want. The fact that such an object is non-bindable should not make it
> unusable actually.

Well, but it's unusable (from the user space app.) indeed. I have
checked it with a simple example and the original overview is enclosed
below as is.

====================
I have cc:'ed Philippe since the code backtrace would be of interest
mainly for him :)

Here is a misbehaviour of the native skin and, well, partially your's :)
The problem is that it allows you creating of an object with a NULL
name, but such an object will not get a record in the registry when
created from the user-mode! This said, one can use NULL-named objects
only from the kernel-mode when there is a direct access to the real
RT_MUTEX structure.

Look,

rt_mutex_create()
{
...
   if (name && *name)
        {
        err = rt_registry_enter(mutex->name,mutex,&mutex->handle,&__mutex_pnode);

and in our case, name == "\0".

So when a creation is completed, mutex->handle == 0!

Then, goes more fun

syscall.c::__rt_mutex_delete() or __rt_mutex_lock()
{
...
 mutex = (RT_MUTEX *)rt_registry_fetch(ph.opaque);

// we know that ph.opaque == 0, so guess what would be returned? Heh,
a handle of the current task! :)


void *rt_registry_fetch (rt_handle_t handle)

{
...
    if (handle == RT_REGISTRY_SELF)	// RT_REGISTRY_SELF == 0. What a
coincidence! :)
	{
	if (!xnpod_primary_p())	// not our case if we are in the primiry mode
            {
            objaddr = NULL;
            goto unlock_and_exit;
            }

	// that's our case

        if (xnpod_current_thread()->magic == RTAI_SKIN_MAGIC)
            {
            objaddr = rtai_current_task();	<=== (*)
            goto unlock_and_exit;
            }
        }
...

So rt_registry_fetch() returns a valid handle but of the current task
and not of a mutex.

Then __rt_mutex_delete() procedes (mutex != NULL) and calls
rt_mutex_delete() which, in turn, calls

  mutex = rtai_h2obj_validate(mutex,RTAI_MUTEX_MAGIC,RT_MUTEX);

    if (!mutex)
        {
        err = rtai_handle_error(mutex,RTAI_MUTEX_MAGIC,RT_MUTEX);
        goto unlock_and_exit;
        }

Since this object is not of RTAI_MUTEX_MAGIC type, the EINVAL error occurs.

So to sum it up.

You have been able to create all your objects but haven't been able to
use them properly. Probably, you are not checking a result of
rt_mutex_lock() otherwise you would be able to get EINVAL error even
yesterday.

Why you can't do it today? Since you have been able to create the
NULL-named objects (that leads to allocating some memory from the
heap) but rt_mutex_delete() failed all the time (ok, I know, who cares
about the proper error checking when deleting :) - the memory was not
freed. So all your heap has gone by today. You haven't reloaded
modules, have you?

So it must be fixed. There must be an explicit prohibition on creating
NULL-named objects from the user-mode.

--
Best regards,
Dmitry Adamushko

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

[Philippe Gerum]

Dmitry Adamushko wrote:
>>>One more thing. I had a discussion with Steven Seeger regarding the use
>>>of NULL-named objects from the user space. I cc'ed you but probably you
>>>were too buzy at that time. The problem is that one may create
>>>successfully a NULL-named object but then there is no way to use it
>>>since all further calls give an error (some funny stuff there indeed :)
>>>
>>>So a simple fix would look like:
>>>
>>>for all rt_OBJECT_create() calls in libnative:
>>>
>>>int rt_mutex_create(RT_MUTEX *mutex, const char *name)
>>>{
>>>
>>>+ if (!name)
>>>+ return -E_SOMETHING; // E_INVAL?
>>>
>>>return XENOMAI_SKINCALL2(__xeno_muxid,
>>>__xeno_mutex_create,
>>>mutex,
>>>name);
>>>}
>>>
>>>What do you think? Or are there any reasons to keep it as is now?
>>>
> 
> 
>>The reason is to allow anonymous objects to be created, so that the descriptor
>>could only be shared by tasks belonging to the same address space if they
>>happen
>>to all have access to the descriptor's memory. Kind of semi-private object if
>>you want. The fact that such an object is non-bindable should not make it
>>unusable actually.
> 
> 
> Well, but it's unusable (from the user space app.) indeed. I have
> checked it with a simple example and the original overview is enclosed
> below as is.
>

As you noticed below, the point is that this feature should be active for 
kernel-based code only; for user-space, we're toast: typical chicken-and-egg 
problem since we need the registry to cross the space boundaries but the 
registry requires a name to index the object first. So yes, we need to check for 
anonymous calls in every service taking a symbolic name in native/syscalls.c, 
and return -EINVAL when applicable.

> ====================
> I have cc:'ed Philippe since the code backtrace would be of interest
> mainly for him :)
> 
> Here is a misbehaviour of the native skin and, well, partially your's :)
> The problem is that it allows you creating of an object with a NULL
> name, but such an object will not get a record in the registry when
> created from the user-mode! This said, one can use NULL-named objects
> only from the kernel-mode when there is a direct access to the real
> RT_MUTEX structure.
> 
> Look,
> 
> rt_mutex_create()
> {
> ...
>    if (name && *name)
>         {
>         err = rt_registry_enter(mutex->name,mutex,&mutex->handle,&__mutex_pnode);
> 
> and in our case, name == "\0".
> 
> So when a creation is completed, mutex->handle == 0!
> 
> Then, goes more fun
> 
> syscall.c::__rt_mutex_delete() or __rt_mutex_lock()
> {
> ...
>  mutex = (RT_MUTEX *)rt_registry_fetch(ph.opaque);
> 
> // we know that ph.opaque == 0, so guess what would be returned? Heh,
> a handle of the current task! :)
> 
> 
> void *rt_registry_fetch (rt_handle_t handle)
> 
> {
> ...
>     if (handle == RT_REGISTRY_SELF)	// RT_REGISTRY_SELF == 0. What a
> coincidence! :)
> 	{
> 	if (!xnpod_primary_p())	// not our case if we are in the primiry mode
>             {
>             objaddr = NULL;
>             goto unlock_and_exit;
>             }
> 
> 	// that's our case
> 
>         if (xnpod_current_thread()->magic == RTAI_SKIN_MAGIC)
>             {
>             objaddr = rtai_current_task();	<=== (*)
>             goto unlock_and_exit;
>             }
>         }
> ...
> 
> So rt_registry_fetch() returns a valid handle but of the current task
> and not of a mutex.
> 
> Then __rt_mutex_delete() procedes (mutex != NULL) and calls
> rt_mutex_delete() which, in turn, calls
> 
>   mutex = rtai_h2obj_validate(mutex,RTAI_MUTEX_MAGIC,RT_MUTEX);
> 
>     if (!mutex)
>         {
>         err = rtai_handle_error(mutex,RTAI_MUTEX_MAGIC,RT_MUTEX);
>         goto unlock_and_exit;
>         }
> 
> Since this object is not of RTAI_MUTEX_MAGIC type, the EINVAL error occurs.
> 
> So to sum it up.
> 
> You have been able to create all your objects but haven't been able to
> use them properly. Probably, you are not checking a result of
> rt_mutex_lock() otherwise you would be able to get EINVAL error even
> yesterday.
> 
> Why you can't do it today? Since you have been able to create the
> NULL-named objects (that leads to allocating some memory from the
> heap) but rt_mutex_delete() failed all the time (ok, I know, who cares
> about the proper error checking when deleting :) - the memory was not
> freed. So all your heap has gone by today. You haven't reloaded
> modules, have you?
> 
> So it must be fixed. There must be an explicit prohibition on creating
> NULL-named objects from the user-mode.
> 
> --
> Best regards,
> Dmitry Adamushko
> 


-- 

Philippe.

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

[Philippe Gerum]

Dmitry Adamushko wrote:

<snip>

> 
> So it must be fixed. There must be an explicit prohibition on creating
> NULL-named objects from the user-mode.
> 

...Or, we might auto-generate some dummy name in native/syscalls.c we would pass 
to the registry when this situation arises, so that anonymous creation and use 
from user-space would still be possible.

-- 

Philippe.

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

[Dmitry Adamushko]

> As you noticed below, the point is that this feature should be active for
> kernel-based code only; for user-space, we're toast: typical chicken-and-egg
> problem since we need the registry to cross the space boundaries but the
> registry requires a name to index the object first. So yes, we need to check for
> anonymous calls in every service taking a symbolic name in native/syscalls.c,
> and return -EINVAL when applicable.

I thought that "libnative" would be a better place since this way we
would avoid the user mode -> kernel mode switch.

> ...Or, we might auto-generate some dummy name in native/syscalls.c we would pass
> to the registry when this situation arises, so that anonymous creation and use
> from user-space would still be possible.

Yep, in this case a name would be a string == object's address, thus
it's unique.

Ok, I'd probably vote for the 2-nd approach.


> --
>
> Philippe.

---
Best regards,
Dmitry

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

[Philippe Gerum]

Dmitry Adamushko wrote:
>>As you noticed below, the point is that this feature should be active for
>>kernel-based code only; for user-space, we're toast: typical chicken-and-egg
>>problem since we need the registry to cross the space boundaries but the
>>registry requires a name to index the object first. So yes, we need to check for
>>anonymous calls in every service taking a symbolic name in native/syscalls.c,
>>and return -EINVAL when applicable.
> 
> 
> I thought that "libnative" would be a better place since this way we
> would avoid the user mode -> kernel mode switch.
> 
> 
>>...Or, we might auto-generate some dummy name in native/syscalls.c we would pass
>>to the registry when this situation arises, so that anonymous creation and use
>>from user-space would still be possible.
> 
> 
> Yep, in this case a name would be a string == object's address, thus
> it's unique.
> 
> Ok, I'd probably vote for the 2-nd approach.
> 

Definitely better since this keeps the semantics consistent across execution spaces.

-- 

Philippe.

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

[Dmitry Adamushko]

On 10/10/05, Philippe Gerum <rpm@xenomai.org> wrote:
> Dmitry Adamushko wrote:
> >>As you noticed below, the point is that this feature should be active for
> >>kernel-based code only; for user-space, we're toast: typical chicken-and-egg
> >>problem since we need the registry to cross the space boundaries but the
> >>registry requires a name to index the object first. So yes, we need to check for
> >>anonymous calls in every service taking a symbolic name in native/syscalls.c,
> >>and return -EINVAL when applicable.
> >
> >
> > I thought that "libnative" would be a better place since this way we
> > would avoid the user mode -> kernel mode switch.
> >
> >
> >>...Or, we might auto-generate some dummy name in native/syscalls.c we would pass
> >>to the registry when this situation arises, so that anonymous creation and use
> >>from user-space would still be possible.
> >
> >
> > Yep, in this case a name would be a string == object's address, thus
> > it's unique.
> >
> > Ok, I'd probably vote for the 2-nd approach.
> >
>
> Definitely better since this keeps the semantics consistent across execution spaces.
>

Ok, will be done.


> --
>
> Philippe.
>

--
Best regards,
Dmitry Adamushko

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

[Dmitry Adamushko]

On 10/10/05, Philippe Gerum <rpm@xenomai.org> wrote:
> Dmitry Adamushko wrote:
> >>As you noticed below, the point is that this feature should be active for
> >>kernel-based code only; for user-space, we're toast: typical chicken-and-egg
> >>problem since we need the registry to cross the space boundaries but the
> >>registry requires a name to index the object first. So yes, we need to check for
> >>anonymous calls in every service taking a symbolic name in native/syscalls.c,
> >>and return -EINVAL when applicable.
> >
> >
> > I thought that "libnative" would be a better place since this way we
> > would avoid the user mode -> kernel mode switch.
> >
> >
> >>...Or, we might auto-generate some dummy name in native/syscalls.c we would pass
> >>to the registry when this situation arises, so that anonymous creation and use
> >>from user-space would still be possible.
> >
> >
> > Yep, in this case a name would be a string == object's address, thus
> > it's unique.
> >
> > Ok, I'd probably vote for the 2-nd approach.
> >
>
> Definitely better since this keeps the semantics consistent across execution spaces.
>

Then we still have a difference in behaviour of the objectes depending
on where they are craeted. I mean, the NULL-named user-space objects
will be displayed under some, well, not-that-informative names (e.g.
0xc3264780) while the kernel-based ones will not have an entry in the
registry (they are really NULL-named).

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.

Then we don't need "*name = '\n';" in syscall.c:__rt_object_create().
But this bit should be removed in any case.

What do you think?

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.

---
Best regards,
Dmitry





--
Best regards,
Dmitry Adamushko

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

[Jim Cromie]

Philippe Gerum wrote:

> Dmitry Adamushko wrote:
>
>>> As you noticed below, the point is that this feature should be 
>>> active for
>>> kernel-based code only; for user-space, we're toast: typical 
>>> chicken-and-egg
>>> problem since we need the registry to cross the space boundaries but 
>>> the
>>> registry requires a name to index the object first. So yes, we need 
>>> to check for
>>> anonymous calls in every service taking a symbolic name in 
>>> native/syscalls.c,
>>> and return -EINVAL when applicable.
>>
>>
>>
>> I thought that "libnative" would be a better place since this way we
>> would avoid the user mode -> kernel mode switch.
>>
>>
>>> ...Or, we might auto-generate some dummy name in native/syscalls.c 
>>> we would pass
>>> to the registry when this situation arises, so that anonymous 
>>> creation and use
>>> from user-space would still be possible.
>>
>>
>>
>> Yep, in this case a name would be a string == object's address, thus
>> it's unique.
>>
>> Ok, I'd probably vote for the 2-nd approach.
>>
>
> Definitely better since this keeps the semantics consistent across 
> execution spaces.
>
maybe we should go as far as formalizing the "stringification" of a 
xenomai object
as a URL:
xeno_queue:0x45abc034
xeno_mutex:0xDEADBEEF
or
xeno:queue:0x0F00BAAB
xeno:mutex:
xeno:shared:TGID=100:0xdeadbeef

it still feels a teeny bit hacky, but the url prefix at least makes its 
use explicit.
In the last example, the url includes TGID=100, the idea being that it would
only be valid for user-space processes that were thread-group 100.

I dunno whether any such objects should get entries in /proc/ipipe/Xenomai*
On one hand, it would seem a decent rendevous point, but not all objects 
should
be globally visible, and its not clear to me which they are.  Anyway, 
reading a
/proc/ipipe/* file is a clumsy way to get addresses of xeno-objects to 
bind to.

thx
jimc

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

[Philippe Gerum]

Dmitry Adamushko wrote:
> On 10/10/05, Philippe Gerum <rpm@xenomai.org> wrote:
> 
>>Dmitry Adamushko wrote:
>>
>>>>As you noticed below, the point is that this feature should be active for
>>>>kernel-based code only; for user-space, we're toast: typical chicken-and-egg
>>>>problem since we need the registry to cross the space boundaries but the
>>>>registry requires a name to index the object first. So yes, we need to check for
>>>>anonymous calls in every service taking a symbolic name in native/syscalls.c,
>>>>and return -EINVAL when applicable.
>>>
>>>
>>>I thought that "libnative" would be a better place since this way we
>>>would avoid the user mode -> kernel mode switch.
>>>
>>>
>>>
>>>>...Or, we might auto-generate some dummy name in native/syscalls.c we would pass
>>>>to the registry when this situation arises, so that anonymous creation and use
>>>
>>>>from user-space would still be possible.
>>>
>>>
>>>Yep, in this case a name would be a string == object's address, thus
>>>it's unique.
>>>
>>>Ok, I'd probably vote for the 2-nd approach.
>>>
>>
>>Definitely better since this keeps the semantics consistent across execution spaces.
>>
> 
> 
> Then we still have a difference in behaviour of the objectes depending
> on where they are craeted. I mean, the NULL-named user-space objects
> will be displayed under some, well, not-that-informative names (e.g.
> 0xc3264780) while the kernel-based ones will not have an entry in the
> registry (they are really NULL-named).
> 
> 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.

> Then we don't need "*name = '\n';" in syscall.c:__rt_object_create().
> But this bit should be removed in any case.
> 
> What do you think?
> 
> 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.

-- 

Philippe.

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

[Dmitry Adamushko]

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.

> --
>
> Philippe.


--
Best regards,
Dmitry Adamushko

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

[Philippe Gerum]

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, otherwise=registered,exported. Looks ok for that 
purpose, since we do need the additional 2nd state here.

-- 

Philippe.

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

[Philippe Gerum]

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.