Hi all,
OK, by now I'll keep using the alternative to use another name for the
elf app/FILE_APPS, as seemingly intended by
chao.an in his commit.
I don't really understand Chaos solution because if the ELF app needs a
bigger stack, which is quite probable because it is a
derivation/evolution of the built-in command with the same name, you'll
have to increase the spawn stack size anyway !?!?
But that's for you pros to decide.
At least seems you got a communication problem here, because nobody had
a notion about the impact of Chaos commit.
Thanks to all & BR,
--
Florian Wehmeyer
TFW Tech-Solutions
On Seg, 2020-05-18 at 06:53 +0000, 安超 wrote:
> Hi Greg,
>
> About the break commit:
>
> commit 9a28ccf836c1b9f0eb5e1163964042eddc207697
> Author: chao.an <anc...@xiaomi.com>
> Date: Fri Feb 21 09:54:47 2020 +0800
>
> nsh/parse: try the builtin configuration first
>
> In the case of enable the BUILTIN_APPS/FILE_APPS at the same
> time, try the builtin list first to ensure that the relevant
> configuration(stacksize, priority) can be set normally.
>
>
> Yes, the change did not consider the case where the file and builtin
> application have the same name,
> but I think it is too violent to directly revert it ...
>
> The original intention of this change is to solve the problem that
> the builtin attribute
> (priority and stack) cannot take effect when both the file and
> builtin application are enabled.
>
> Think about what should we do if an file application needs a stack
> size larger than spawn?
>
> For example,
> 1. Suppose the program needs a large stack size:
>
> diff --git a/examples/hello/hello_main.c
> b/examples/hello/hello_main.c
> index 953597a..7b832b6 100644
> --- a/examples/hello/hello_main.c
> +++ b/examples/hello/hello_main.c
> @@ -50,6 +50,10 @@
>
> int main(int argc, FAR char *argv[])
> {
> + char dummy_array[CONFIG_EXAMPLES_HELLO_STACKSIZE - 2048];
> +
> + memset(dummy_array, 0, sizeof(dummy_array));
> printf("Hello, World!!\n");
> + sleep(10000);
> return 0;
> }
>
> 2. Check the stack usage, since the default
> CONFIG_EXAMPLES_HELLO_STACKSIZE
> follows CONFIG_TASK_SPAWN_DEFAULT_STACKSIZE are 8192, so there is
> no problem
>
> nsh> Hello, World!!
> ps
> PID PPID PRI POLICY TYPE NPX
> STATE EVENT SIGMASK STACK USED FILLED COMMAND
> 0 0 0 FIFO Kthread N-- Ready 00000000
> 000000 000000 0.0% Idle Task
> 2 1 100 FIFO Task --- Running 00000000
> 008160 002236 27.4% /system/bin/nsh
> 3 2 100 FIFO Task --- Waiting Signal 00000000
> 008176 007240 88.5%! hello
> nsh>
>
> 3. But if we change the stack size to 20480, see what happens:
> +CONFIG_EXAMPLES_HELLO_STACKSIZE=20480
>
> nsh> hello &
> hello [0:100]
> nsh> Hello, World!!
> Segmentation fault (core dumped)
>
> Size 8192 is still used in posix_spawn, which will cause a loadable
> application stack overflow,
> Of course you will say that in this case we can increase the stack
> size of posix_spawn,
> but in ths case, we will waste more stack space for small
> applications.
>
> -------------------------------------------------------------------
> ----------------
>
> To resolve this issue I sent a pull request and made a simple fix:
>
> https://github.com/apache/incubator-nuttx-apps/pull/258
>
> 1. Bring back the BUILTIN_APPS / FILE_APPS calling sequence,
> 2. Try FILE_APPS first in the case of builtin:
>
> After patch:
>
> nsh> hello &
> hello [3:100]
> nsh> Hello, World!!
> ps
> PID PPID PRI POLICY TYPE NPX
> STATE EVENT SIGMASK STACK USED FILLED COMMAND
> 0 0 0 FIFO Kthread N-- Ready 00000000
> 000000 000000 0.0% Idle Task
> 2 1 100 FIFO Task --- Running 00000000
> 008160 002300 28.1% /system/bin/nsh
> 3 2 100 FIFO Task --- Waiting Signal 00000000
> 020464 019528 95.4%! hello
>
> It is make sense to use the builtin attribute for applications if we
> enabled the BUILTIN_APPS / FILE_APPS at the same time.
>
> BRs,
> ________________________________________
> 发件人: Gregory Nutt <spudan...@gmail.com>
> 发送时间: 2020年5月18日 6:59
> 收件人: dev@nuttx.apache.org
> 主题: [External Mail]Re: heap malloc when executing binary/elf file
>
> Hi, Brennan,
> >
> > >
> > > >
> > > > I just created incubator-nuttx-apps PR 255 that should restore
> > > > the
> > > > correct behavior:
> > > https://github.com/apache/incubator-nuttx-apps/pull/255
> > >
> > > This is a major bug and, if we release do a bugfix release
> > > sometime,
> > > this should be included.
> > >
> > I was thinking the same. I created a label backport/9.0 that we can
> > attach
> > to PRs that we think should be backported to a bugfix release. I
> > suspect
> > especially with the amount of changes that went into this release
> > it won't
> > be the only one.
> > @btashton btashton added the backport/9.0 label 10 seconds ago
> I agree that this should be backported. It is a major bug; a
> complete
> loss of important functionality.
>
> But let's let is simmer for a bit to make sure that it does the job
> and
> doesn't introduce new issues.
>
> Is this your preferred way of handling bugs? Adding the backport/9.0
> tag? Optionally, we could just add the commits to the tip of the
> releases/9.0 branch where they could be picked up at some later
> bugfix
> release.
>
> I have no preference. At some point, I think we also have to say
> that
> 9.0.x "is what it is". Or do you see us continuing to maintain the
> release forever? In the dark, pre-Apache past, I had about a two
> week
> period after the release after which it "is what it is". I
> distrubuted
> patches up until that point. What policy would you recommend?
>
> Greg
>
>
>
> #/******本邮件及其附件含有小米公司的保密信息，仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用（包括但不限于全
> 部或部分地泄露、复制、或散发）本邮件中的信息。如果您错收了本邮件，请您立即电话或邮件通知发件人并删除本邮件！ This e-mail
> and its attachments contain confidential information from XIAOMI,
> which is intended only for the person or entity whose address is
> listed above. Any use of the information contained herein in any way
> (including, but not limited to, total or partial disclosure,
> reproduction, or dissemination) by persons other than the intended
> recipient(s) is prohibited. If you receive this e-mail in error,
> please notify the sender by phone or email immediately and delete
> it!******/#
