Re: Duplicate task_spawn()
Thanks for reporting this issue.
There a method to simplify the implementation and keep the compatibility,
please try these patch:
https://github.com/apache/incubator-nuttx/pull/4027
https://github.com/apache/incubator-nuttx-apps/pull/791
On Thu, Jul 1, 2021 at 8:40 PM Sebastien Lorquet
wrote:
> I see that around the time of this discussion the userland API of
> task_spawn has been changed in incompatible ways.
>
> by this commit:
>
>
> https://github.com/apache/incubator-nuttx-apps/commit/58293abb8e896bb04f3a76bf8b48206debe68f26?branch=58293abb8e896bb04f3a76bf8b48206debe68f26=unified
>
> My apps, cloned from a nuttx repo that did not have this change, are
> suddenly broken in places I do not even care about.
>
> An example:
>
> In file included from exec_builtin.c:49:0:
> /home/slo/nuttx/include/spawn.h:150:5: note: expected 'char * const*'
> but argument is of type 'posix_spawnattr_t * {aka struct
> posix_spawnattr_s * '
> int task_spawn(FAR const char *name, main_t entry,
> ^~
> exec_builtin.c:195:13: error: too many arguments to function 'task_spawn'
> ret = task_spawn(, builtin->name, builtin->main, _actions,
>
>
> This is rather annoying. On the long term, this project becomes unstable
> in multiple ways.
>
> I suggest that user API breaks are documented in the same place as was
> used for other breaking changes.
>
> This is how I feel right now: https://www.youtube.com/watch?v=_LueFhQk5hg
>
>
> Sebastien
>
>
> Le 01/06/2020 à 16:43, Gregory Nutt a écrit :
> > On 5/30/2020 1:27 AM, Yang Chung Fan wrote:
> >> Hi,
> >>
> >> Did anyone also noticed that when building with CONFIG_LIB_SYSCALL=y,
> >> the linker is unhappy about the duplicated task_spawn() symbols?
> >>
> >> One of them is in sched/ and other one is in libs/libc.
> >>
> > PR 1168 should take care of this.
> >
>
Re: Duplicate task_spawn()
I see that around the time of this discussion the userland API of
task_spawn has been changed in incompatible ways.
by this commit:
https://github.com/apache/incubator-nuttx-apps/commit/58293abb8e896bb04f3a76bf8b48206debe68f26?branch=58293abb8e896bb04f3a76bf8b48206debe68f26=unified
My apps, cloned from a nuttx repo that did not have this change, are
suddenly broken in places I do not even care about.
An example:
In file included from exec_builtin.c:49:0:
/home/slo/nuttx/include/spawn.h:150:5: note: expected 'char * const*'
but argument is of type 'posix_spawnattr_t * {aka struct
posix_spawnattr_s * '
int task_spawn(FAR const char *name, main_t entry,
^~
exec_builtin.c:195:13: error: too many arguments to function 'task_spawn'
ret = task_spawn(, builtin->name, builtin->main, _actions,
This is rather annoying. On the long term, this project becomes unstable
in multiple ways.
I suggest that user API breaks are documented in the same place as was
used for other breaking changes.
This is how I feel right now: https://www.youtube.com/watch?v=_LueFhQk5hg
Sebastien
Le 01/06/2020 à 16:43, Gregory Nutt a écrit :
On 5/30/2020 1:27 AM, Yang Chung Fan wrote:
Hi,
Did anyone also noticed that when building with CONFIG_LIB_SYSCALL=y,
the linker is unhappy about the duplicated task_spawn() symbols?
One of them is in sched/ and other one is in libs/libc.
PR 1168 should take care of this.
Re: Duplicate task_spawn()
On 5/30/2020 1:27 AM, Yang Chung Fan wrote: Hi, Did anyone also noticed that when building with CONFIG_LIB_SYSCALL=y, the linker is unhappy about the duplicated task_spawn() symbols? One of them is in sched/ and other one is in libs/libc. PR 1168 should take care of this.
RE: Duplicate task_spawn()
CONFIG_LIB_SYSCALL is also useful in FLAT build in some special case because calling kernel function through syscall can avoid to patch the relocation table(like ROMFS+NXFLAT). > -Original Message- > From: Gregory Nutt > Sent: Sunday, May 31, 2020 12:07 AM > To: dev@nuttx.apache.org > Subject: Re: Duplicate task_spawn() > > > > Any static should be conditioned on CONFIG_LIB_SYSCALL for the > > task_spawn() version in sched/task/task_spawn.c, however, that is not > > really necessary either because that version is not linked into the > > same binary as is the version in libs/libc/spawn. > > > > I suppose a user could enable CONFIG_LIB_SYSCALL in a FLAT build. Then > > both would be linked into the same blob, but that is kind of a useless > > configuration. > > > So, I think the preferred fix would simply to make CONFIG_LIB_SYSCALL > dependent on !CONFIG_BUILD_FLAT in the Kconfig file. >
Re: Duplicate task_spawn()
> > Any static should be conditioned on CONFIG_LIB_SYSCALL for the > task_spawn() version in sched/task/task_spawn.c, however, that is not > really necessary either because that version is not linked into the same > binary as is the version in libs/libc/spawn. > > I suppose a user could enable CONFIG_LIB_SYSCALL in a FLAT build. Then > both would be linked into the same blob, but that is kind of a useless > configuration. Yes, this is what my current config has. I am trying to port my cRTOS design to newer nuttx. While running in FLAT build, I load Linux binaries in SystemV ABI and hook the system calls redirect them to Nuttx APIs. Of course, I can do this in a kernel build, but I want to keep things simple. I agree that using system calls in a FLAT build is quite meaning less. However, I think we can somehow keep this flexibility for exploiting for binary compability? -- Yang 2020年5月31日(日) 1:07 Gregory Nutt : > > > > Any static should be conditioned on CONFIG_LIB_SYSCALL for the > > task_spawn() version in sched/task/task_spawn.c, however, that is not > > really necessary either because that version is not linked into the > > same binary as is the version in libs/libc/spawn. > > > > I suppose a user could enable CONFIG_LIB_SYSCALL in a FLAT build. > > Then both would be linked into the same blob, but that is kind of a > > useless configuration. > > > So, I think the preferred fix would simply to make CONFIG_LIB_SYSCALL > dependent on !CONFIG_BUILD_FLAT in the Kconfig file. > > -- Yang Chung Fan (楊宗凡) (ヤン ゾン ファン) Member of Softlab, Tsukuba University , Japan Email: sonic.tw...@gmail.com
Re: Duplicate task_spawn()
Any static should be conditioned on CONFIG_LIB_SYSCALL for the task_spawn() version in sched/task/task_spawn.c, however, that is not really necessary either because that version is not linked into the same binary as is the version in libs/libc/spawn. I suppose a user could enable CONFIG_LIB_SYSCALL in a FLAT build. Then both would be linked into the same blob, but that is kind of a useless configuration. So, I think the preferred fix would simply to make CONFIG_LIB_SYSCALL dependent on !CONFIG_BUILD_FLAT in the Kconfig file.
Re: Duplicate task_spawn()
On 5/30/2020 9:22 AM, Gregory Nutt wrote:
On 5/30/2020 3:44 AM, Xiang Xiao wrote:
It seems that the version inside sched/task/task_spawn.c need become
a static function.
I think that the version of task_spawn in lib/libc/spawn need to exist
only if CONFIG_LIB_SYSCALL is selected. In that case, the one in
sched/task/task_spawn.c should be static (really doesn't matter in
that case because they are build into separate blobs).
The version of task_spawn.c in libs/libc/spawn simply marshals the
parameters into a structure and calls nx_task_spawn(). If
CONFIG_LIB_SYSCALL is defined then nx_task_spawn() will un-marshal the
data can call the really task spawn. This nonsense is only necessary
because task_spawn has 8 parameters and the maximum number of
parameters in a system call is only 6.
Without syscalls: Application should call directly in task_spawn() in
sched/task/task_spawn.c
With syscalls: Application should call the marshalling task_spawn()
in libs/libc/spawn/lib_task_spawn.c -> That will call the
autogenerated nx_task_spawn() proxy -> And generate a system call ->
The system call will the unmarshalling nx_task_spawn() in
sched/task/task_spawn.c -> Which will, finally, call the real
task_spawn().
I will add that condition logic via a PR.
Greg
Update:
No changes are required in libs/libc/spawn because the conditional logic
is already in place. in Make.defs:
ifneq ($(CONFIG_BUILD_KERNEL),y)
CSRCS += lib_psa_getstacksize.c lib_psa_setstacksize.c
ifeq ($(CONFIG_LIB_SYSCALL),y)
CSRCS += lib_task_spawn.c
endif
endif
In libs/libc/spawn/task_spawn.c:
#if defined(CONFIG_LIB_SYSCALL) && !defined(CONFIG_BUILD_KERNEL)
Similarly in sched/task/task_spawn.c:
#ifdef CONFIG_LIB_SYSCALL
int nx_task_spawn(FAR const struct spawn_syscall_parms_s *parms)
{
...
}
#endif
Any static should be conditioned on CONFIG_LIB_SYSCALL for the
task_spawn() version in sched/task/task_spawn.c, however, that is not
really necessary either because that version is not linked into the same
binary as is the version in libs/libc/spawn.
I suppose a user could enable CONFIG_LIB_SYSCALL in a FLAT build. Then
both would be linked into the same blob, but that is kind of a useless
configuration.
The side-effect of making task_spawn() static is that it then cannot be
used within the OS. But as far as I can tell, nothing in the OS itself
currently uses task_spawn() so I think it is safe to make it
conditionally static. But that only protects from duplicate symbols in
the useless case mentioned above.
Greg
Re: Duplicate task_spawn()
On 5/30/2020 3:44 AM, Xiang Xiao wrote: It seems that the version inside sched/task/task_spawn.c need become a static function. I think that the version of task_spawn in lib/libc/spawn need to exist only if CONFIG_LIB_SYSCALL is selected. In that case, the one in sched/task/task_spawn.c should be static (really doesn't matter in that case because they are build into separate blobs). The version of task_spawn.c in libs/libc/spawn simply marshals the parameters into a structure and calls nx_task_spawn(). If CONFIG_LIB_SYSCALL is defined then nx_task_spawn() will un-marshal the data can call the really task spawn. This nonsense is only necessary because task_spawn has 8 parameters and the maximum number of parameters in a system call is only 6. Without syscalls: Application should call directly in task_spawn() in sched/task/task_spawn.c With syscalls: Application should call the marshalling task_spawn() in libs/libc/spawn/lib_task_spawn.c -> That will call the autogenerated nx_task_spawn() proxy -> And generate a system call -> The system call will the unmarshalling nx_task_spawn() in sched/task/task_spawn.c -> Which will, finally, call the real task_spawn(). I will add that condition logic via a PR. Greg
RE: Duplicate task_spawn()
It seems that the version inside sched/task/task_spawn.c need become a static function. > -Original Message- > From: Yang Chung Fan > Sent: Saturday, May 30, 2020 3:27 PM > To: dev@nuttx.apache.org > Subject: Duplicate task_spawn() > > Hi, > > Did anyone also noticed that when building with CONFIG_LIB_SYSCALL=y, the > linker is unhappy about the duplicated task_spawn() > symbols? > > One of them is in sched/ and other one is in libs/libc. > > -- > Yang
