Skip to content

Conversation

@JianyuWang0623
Copy link
Contributor

@JianyuWang0623 JianyuWang0623 commented Oct 17, 2025

Summary

Related PR:

Add NuttX Init ("NxInit")

This component (abbreviated as "NxInit") is specifically developed for NuttX and intended for system initialization. While we have adopted the Android Init Language among various syntax options, this is a brand-new implementation—it is not a port or variant of Android Init.

Refer to the implementation of Android Init Language, consists of five broad classes of statements: Actions, Commands, Services, Options, and Imports.

Actions support two types of triggers: event and action. Action triggers also support runtime triggering. Services support lifecycle management, including automatic restart (at specified intervals), and starting/stopping individually or by class. Import supports files or directories, and we may add a static method in the future. The following are some differences:

  1. The Android Init Language treats lines starting with # as comments, while we use a preprocessor to handle comments.
  2. For action commands, we can omit "exec" and directly execute built-in apps or nsh builtins.
  3. Regarding the property service, users can either adapt it by self or directly use the preset NVS-based properties.
  4. Only part of standard action commands and service options are implemented currlently.

To enable system/init:

-CONFIG_INIT_ENTRYPOINT="nsh_main"
+CONFIG_INIT_ENTRYPOINT="init_main"
+CONFIG_SYSTEM_NXINIT=y

For format and additional details, refer to: https://android.googlesource.com/platform/system/core/+/master/init/README.md

Add class start/stop for service

Usage:

      class_start <classname>
      class_stop <classname>

Add NSH builtin support for action

Enable CONFIG_SYSTEM_SYSTEM to support nsh builtins, which depends on nsh.

Warning for long commands

If the command of an action takes too long (greater than CONFIG_SYSTEM_NXINIT_ACTION_WARN_SLOW milliseconds, 50 ms by default), a warning log will be output for analysis and debugging.

For example:

  1. sleep 1
     [    0.340000] [ 3] [ 0] init_main: executing NSH command 'sleep 1'
     [    1.360000] [ 3] [ 0] init_main: NSH command 'sleep 1' exited 0
   > [    1.360000] [ 3] [ 0] init_main: command 'sleep' took 1020 ms
  1. hello (add sleep(1) to examples/hello)
     [    1.390000] [ 3] [ 0] init_main: executed command 'hello' pid 14
     [    1.390000] [ 3] [ 0] init_main: waiting 'hello' pid 14
     Hello, World!!
   > [    2.400000] [ 3] [ 0] init_main: command 'hello' pid 14 took 1010 ms
     [    2.400000] [ 3] [ 0] init_main: command 'hello' pid 14 exited status 0

Add oneshot support for service

Add support for the oneshot option to the service.

Test

  • RC
    service telnet telnetd
        class test
  >     oneshot
        restart_period 3000
  • Runtime
   [    0.150000] [ 3] [ 0] init_main: == Dump Services ==
   ...
   [    0.160000] [ 3] [ 0] init_main: Service 0x40486aa8 name 'telnet' path 'telnetd'
   [    0.160000] [ 3] [ 0] init_main:   pid: 0
   [    0.160000] [ 3] [ 0] init_main:   arguments:
   [    0.160000] [ 3] [ 0] init_main:       [0] 'service'
   [    0.160000] [ 3] [ 0] init_main:       [1] 'telnet'
   [    0.160000] [ 3] [ 0] init_main:       [2] 'telnetd'
   [    0.160000] [ 3] [ 0] init_main:   classes:
   [    0.160000] [ 3] [ 0] init_main:     'test'
   [    0.170000] [ 3] [ 0] init_main:   restart_period: 3000
   [    0.170000] [ 3] [ 0] init_main:   reboot_on_failure: -1
   [    0.170000] [ 3] [ 0] init_main:   flags:
 > [    0.170000] [ 3] [ 0] init_main:     'oneshot'
   ...
   [    0.370000] [ 3] [ 0] init_main: starting service 'telnet' ...
   [    0.380000] [ 3] [ 0] init_main: service 'telnet' flag 0x2 add 0x4
   [    0.380000] [ 3] [ 0] init_main:   +flag 'running'
   [    0.380000] [ 3] [ 0] init_main: service 'telnet' flag 0x6 add 0x8
   [    0.380000] [ 3] [ 0] init_main:   -flag 'restarting'
   [    0.380000] [ 3] [ 0] init_main: service 'telnet' flag 0x6 add 0x1
   [    0.380000] [ 3] [ 0] init_main:   -flag 'disabled'
   [    0.380000] [ 3] [ 0] init_main: started service 'telnet' pid 9
   ...
   nsh> kill -9 9
   nsh> [    7.350000] [ 3] [ 0] init_main: service 'telnet' flag 0x6 add 0x4
   [    7.350000] [ 3] [ 0] init_main:   -flag 'running'
   [    7.350000] [ 3] [ 0] init_main: service 'telnet' flag 0x2 add 0x80000001
   [    7.350000] [ 3] [ 0] init_main:   +flag 'disabled'
   [    7.350000] [ 3] [ 0] init_main:   +flag 'remove'
   [    7.350000] [ 3] [ 0] init_main: service 'telnet' pid 9 exited status 1
 > [    7.360000] [ 3] [ 0] init_main: removing service 'telnet' ...

Add exec_start support for action

Format: exec_start <service>

Start the specified service and pause the processing of any additional initialization commands until the service completes its execution. This command operates similarly to the exec command; the key difference is that it utilizes an existing service definition rather than requiring the exec argument vector.

This feature is particularly intended for use with the reboot_on_failure built-in command to perform all types of essential checks during system boot.

Add reboot_on_failure for service

Add support for the reboot_on_failure option to the service.

When the execution of a command within a certain action fails (returning a non-zero status code), NxInit will continue to execute subsequent commands or actions and will not proactively terminate the startup process. To implement the functionality of "terminating the startup process after a command execution fails", there are two methods:

  1. Execute conditional statements (if/then/else/fi) via exec command, but this depends on sh:
    on init
        exec -- set +e; \
                mount -t fatfs /dev/data /data ; \
                if [ $? -ne 0 ] ; \
                then \
                  echo "failed" ; \
                  reboot ; \
                else \
                  echo "succeed" ; \
                fi;
    
  2. Via service's oneshot + reboot_on_failure:
    Although the example uses sh, it does not depend on it and can be replaced with any other Builtin Apps.
    on init
        exec_start mkdir_tmp
        ls /tmp
    
    service mkdir_tmp sh -c "mkdir /tmp"
        reboot_on_failure 0
        oneshot
    

Test

  • RC
    service console sh
        class core
        override
  >     reboot_on_failure 0
        restart_period 10000
  • Runtime
    [    0.150000] [ 3] [ 0] init_main: == Dump Services ==
    ...
    [    0.170000] [ 3] [ 0] init_main: Service 0x40486ea0 name 'console' path 'sh'
    [    0.170000] [ 3] [ 0] init_main:   pid: 0
    [    0.170000] [ 3] [ 0] init_main:   arguments:
    [    0.170000] [ 3] [ 0] init_main:       [0] 'service'
    [    0.170000] [ 3] [ 0] init_main:       [1] 'console'
    [    0.170000] [ 3] [ 0] init_main:       [2] 'sh'
    [    0.170000] [ 3] [ 0] init_main:   classes:
    [    0.170000] [ 3] [ 0] init_main:     'core'
    [    0.170000] [ 3] [ 0] init_main:   restart_period: 10000
  > [    0.170000] [ 3] [ 0] init_main:   reboot_on_failure: 0
    [    0.170000] [ 3] [ 0] init_main:   flags:
    [    0.170000] [ 3] [ 0] init_main:     'override'
    ...
    [    0.380000] [ 3] [ 0] init_main: started service 'console' pid 4
    ...
    nsh> kill -9 4
    [    8.060000] [ 3] [ 0] init_main: service 'console' flag 0x20000004 add 0x4
    [    8.060000] [ 3] [ 0] init_main:   -flag 'running'
    [    8.060000] [ 3] [ 0] init_main: service 'console' flag 0x20000000 add 0x8
    [    8.060000] [ 3] [ 0] init_main:   +flag 'restarting'
  > [    8.060000] [ 3] [ 0] init_main: Error reboot on failure of service 'console' reason 0

Impact

This component (abbreviated as "NxInit") is specifically developed for NuttX and intended for system initialization. While we have adopted the Android Init Language among various syntax options, this is a brand-new implementation—it is not a port or variant of Android Init.

Testing

  • Please see logs above for selftest <--- @JianyuWang0623 maybe the test logs could be placed here instead?
  • CI

linguini1
linguini1 previously approved these changes Oct 17, 2025
Copy link
Contributor

@linguini1 linguini1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The testing looks really thorough, great job! This is a very large PR though, with 7 commits. Is there any way it can broken up? Not sure if @jerpelea would find that easier when generating release notes, or if that only applies to the NuttX kernel. Otherwise looks good from me.

@JianyuWang0623
Copy link
Contributor Author

The testing looks really thorough, great job! This is a very large PR though, with 7 commits. Is there any way it can broken up? Not sure if @jerpelea would find that easier when generating release notes, or if that only applies to the NuttX kernel. Otherwise looks good from me.

@linguini1 Yes, this PR is quite large because it includes a newly added feature. On the premise of ensuring complete functionality, I have split the commits as much as possible, but the review of the first commit - "system/init: Add system init" - will still take a lot of effort. Fortunately, the review of the remaining commits will be much easier.

Copy link
Contributor

@acassis acassis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice work @JianyuWang0623 !!!
Please add an Documentation about this system init. Although a documentation for Android exists, NuttX will need a Documentation/ with specific case uses and configurations

@cederom
Copy link
Contributor

cederom commented Oct 19, 2025

Thank you @JianyuWang0623 looks really nice :-) I am a bit afraid that "system" and "init" are too broad terms and may cause confusion.. maybe it would be better to call them "Android System Init" and name config variables something like CONFIG_GOOGLE_ANDROID_SYSTEM_INIT or CONFIG_ANDROID_SYSTEM_INIT or CONFIG_GOOGLE_SYSTEM_INIT in place of CONFIG_SYSTEM_INIT whis is not really self explanatory? :-P First comers would then know this functionality covers Google Android System Init.. unless it is forbidden to use Google and Android names as trademarked? :-P

@cederom
Copy link
Contributor

cederom commented Oct 19, 2025

nice work @JianyuWang0623 !!! Please add an Documentation about this system init. Although a documentation for Android exists, NuttX will need a Documentation/ with specific case uses and configurations

+1 :-)

@acassis
Copy link
Contributor

acassis commented Oct 20, 2025

@xiaoxiang781216 some months ago you commented about planing to add support to SystemV Init. Is this Android Init a replacement or do you still planing to add support to SystemV Init?

@JianyuWang0623
Copy link
Contributor Author

Thank you @JianyuWang0623 looks really nice :-) I am a bit afraid that "system" and "init" are too broad terms and may cause confusion.. maybe it would be better to call them "Android System Init" and name config variables something like CONFIG_GOOGLE_ANDROID_SYSTEM_INIT or CONFIG_ANDROID_SYSTEM_INIT or CONFIG_GOOGLE_SYSTEM_INIT in place of CONFIG_SYSTEM_INIT whis is not really self explanatory? :-P First comers would then know this functionality covers Google Android System Init.. unless it is forbidden to use Google and Android names as trademarked? :-P

@cederom The configuration is named SYSTEM_INIT to align with the source code directory structure.
To enhance usability and reduce the learning barrier, we’ve ensured its syntax is highly compatible with Android Init Language. Though we gained valuable insights from Android during design and implementation, we opted for a tailored, simplified version rather than direct porting. This decision focused on optimizing code size, RAM usage, and symbol storage locations; detailed metrics will be included in the NuttX kernel repository’s documentation.

@JianyuWang0623
Copy link
Contributor Author

nice work @JianyuWang0623 !!! Please add an Documentation about this system init. Although a documentation for Android exists, NuttX will need a Documentation/ with specific case uses and configurations

@acassis @cederom Thanks! Good call—we definitely need to add the docs. Could you please review the pull request? apache/nuttx#17215

@xiaoxiang781216
Copy link
Contributor

xiaoxiang781216 commented Oct 20, 2025

@xiaoxiang781216 some months ago you commented about planing to add support to SystemV Init. Is this Android Init a replacement or do you still planing to add support to SystemV Init?

we decide to use Andriod init style since it's more simpler and clear. BTW, the copyright used by Andriod is same as NuttX.

@cederom
Copy link
Contributor

cederom commented Oct 20, 2025

@xiaoxiang781216 some months ago you commented about planing to add support to SystemV Init. Is this Android Init a replacement or do you still planing to add support to SystemV Init?

we decide to use Andriod init style since it's more simpler and clear. BTW, the copyright used by Andriod is same as NuttX.

Thanks @xiaoxiang781216 then I would advise to use "Android System Init" naming conventions in the all identifiers so things are self-explanatory and people know at first glance which system init standard is used? What do you think guys ? :-)

Copy link
Contributor

@cederom cederom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @JianyuWang0623 amazing work :-)

I would advise using "Android System Init" naming convention as there are several different init types out there and the name / variables would be self-explanatory and maybe one day add other init types.

But if others are fine with this assumption that "system init" is "android system init" then I will follow :-)

list_for_every_entry_continue(am->current, &am->actions,
struct action_s, node)
{
if (am->current->event && !strcmp(am->current->event,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please consider using strncmp() instead to avoid stack overflow

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@acassis Thank you for the reminder. We have also considered this issue. However, the good news is that the values of both am->current->event and am->events[i] are obtained from the strdup() function, which normally includes a terminating null byte.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

strncmp with a max length can still protect from bogus user input. I recommend doing something here. the boot process is critical and a user should not be able to mess things up with bogus data injected in the filesystem.

Copy link
Contributor

@cederom cederom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a feedback after dev@ mailing list discussion, thank you for understanding / considering @JianyuWang0623 :

  1. We clearly need to use "Android System Init" naming here and update all kconfig/global variables names to clearly distinguish from current NuttX Init, Android System Init, and SystemV Init (there is sill a change it will show up here one day), maybe others. "system_system" or "system_init" or "init_main" is really confusing and not self-explanatory, does not even point to "android_system_init". I know this is a bit longer but will provide important context and clear separation between completely different functionalities / implementations.
  2. We need better documentation for this "Android System Init" as well as "Fastboot". These functionalities are welcome, people want to use it, but users / developers ask questions like: Why do we need another init system? What should happen with the previous ones? What does this new init system solve? How this relates to Fastboot? When, why, and how can I use it? What are best use cases with some examples? Please consider approach like explaining and convincing someone who wants to use it but has zero knowledge about the solution.

@JianyuWang0623
Copy link
Contributor Author

JianyuWang0623 commented Oct 21, 2025

Just a feedback after dev@ mailing list discussion, thank you for understanding / considering @JianyuWang0623 :

  1. We clearly need to use "Android System Init" naming here and update all kconfig/global variables names to clearly distinguish from current NuttX Init, Android System Init, and SystemV Init (there is sill a change it will show up here one day), maybe others. "system_system" or "system_init" or "init_main" is really confusing and not self-explanatory, does not even point to "android_system_init". I know this is a bit longer but will provide important context and clear separation between completely different functionalities / implementations.
  2. We need better documentation for this "Android System Init" as well as "Fastboot". These functionalities are welcome, people want to use it, but users / developers ask questions like: Why do we need another init system? What should happen with the previous ones? What does this new init system solve? How this relates to Fastboot? When, why, and how can I use it? What are best use cases with some examples? Please consider approach like explaining and convincing someone who wants to use it but has zero knowledge about the solution.

@cederom The following is part of the reply.

Why do we need another init system? What should happen with the previous ones? What does this new init system solve?

As I supplemented in the data for the PR apache/nuttx#17216 , NSH, when used as an initialization program, may seem relatively bloated in scenarios where code size is a major concern. NSH and Init can coexist, allowing users to choose based on their own needs. Init addresses issues such as the excessive code size of NSH when used as an initialization program, and its lack of management (e.g., restarting) for daemons.

Config Changes ELF Size(bytes)
defconfig 7,008,056
+ETC_ROMFS 7,066,048
+INIT 7,127,536
-TELENT 7,127,024
-NSH_LIBRARY
-SYSTEM_NSH
6,509,392

For the ELF file, Init requires 61,488 bytes, while NSH and NSH_LIBRARY together require 617,632 bytes. Switching from NSH to Init can save a significant amount of space(556,144 bytes).

How this relates to Fastboot?

Init and Fastboot have nothing to do with each other; it's just that a daemon is needed as an example of a service.

When, why, and how can I use it?

Here are some simple examples: When code size is a major concern and no shell is needed, replacing NSH with Init can save a significant amount of space; management functions such as restarting are required when a daemon fails to start or exits abnormally; and so on.

What are best use cases with some examples?

An example has already been added to apache/nuttx#17215.



Some supplementary explanations:

  • The naming "Init" is based on its functional perspective;
  • The Kconfig naming "SYSTEM_INIT" follows the conventions of existing components, with reference to the code path "apps/system/init" where "SYSTEM" corresponds to the "system" directory;
  • Compatibility with Android Init Language is considered for ease of use;
  • However, we are not directly porting Android Init, but rather implementing a new, lightweight Init that is compatible with most of its syntax.

FAR struct action_cmd_s *running;
int pid_running;
#if defined(CONFIG_SYSTEM_INIT_ACTION_WARN_SLOW) && \
CONFIG_SYSTEM_INIT_ACTION_WARN_SLOW > 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are always inline-ing the CONFIG_SYSTEM_INIT_ACTION_WARN_SLOW > 0 in every check in the c files.
Would it not help to assert that here, in the header file? something like

#if defined(CONFIG_SYSTEM_INIT_ACTION_WARN_SLOW) && \
    CONFIG_SYSTEM_INIT_ACTION_WARN_SLOW > 0
#  error Threshold should be positive
#endif

Any other piece of code we can only use #if defined(CONFIG_SYSTEM_INIT_ACTION_WARN_SLOW)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tinnedkarma A value of zero is not an exception; rather, like being undefined, it indicates that this function is disabled.

SYSTEM_INIT_ACTION_WARN_SLOW

Value The function
Undefined Disable
Zero Disable
Greater than zero Enable

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I've made a mistake by copying the lines.
There should be CONFIG_SYSTEM_INIT_ACTION_WARN_SLOW < 0.
In my example above, I forgot to change the sign from ">" to "<".
Anyway, maybe I did not understand correctly, we need to make sure the option have a positive integer for the value, right?

}

#ifdef CONFIG_SYSTEM_INIT_DEBUG
void init_dump_actions(FAR struct list_node *head)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't this be moved directly to init.c?
It's only used in init.c, and it does not depend on anything from action.c specifically

  • head is passed from init.c
  • action is defined inside function scope
  • struct action_s is the literal type
  • node is the literal member

Nothing from the action.c is referenced here. Am I wrong?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tinnedkarma You're right, but from a functional standpoint, this is part of the action.

* Private Functions
****************************************************************************/

static int cmd_class_start(FAR struct action_manager_s *am,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function just calls another function.
I suggest making it static inline

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tinnedkarma I'm not sure if you've read the entire file.

static const struct cmd_map_s g_builtin[] =
{
  {"class_start", 2, 2, cmd_class_start},
  {"class_stop", 2, 2, cmd_class_stop},
  {"exec", 3, 99, cmd_exec},
  {"exec_start", 2, 2, cmd_exec_start},
  {"start", 2, 2, cmd_start},
  {"stop", 2, 2, cmd_stop},
  {"trigger", 2, 2, cmd_trigger},
};

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, sorry, I may have mislead you with my previous suggestion.
I suggested to also add inline to the function signature, not drop the return value.

A function call creates a new stack, so here we have a stack for the cmd_class_start, and immediately after we create a new stack for init_service_start_by_class. By inline-ing the cmd_class_start we can avoid creating a useless stack.

Correct me if I am wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tinnedkarma The function is only used for function pointers. What I'm wondering is whether 'inline' has any practical significance?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JianyuWang0623 To my knowledge yes, It's also my point, there is no "internal" logic to this function, so we could avoid incrementing the stack depth with another push-ret if it does not offer any practical significance.
Anyway, maybe another opinion will help.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this function is called through pointer, inline can't help here anyway.

if (ret != 0)
{
#ifdef CONFIG_SYSTEM_SYSTEM
char cmd[CONFIG_SYSTEM_INIT_RC_LINE_MAX];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is also a pointer, should we add FAR?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

char array doesn't add FAR since it allocate in stack directly

#define init_err(...)
#endif

#define init_log(p, ...) \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason why this should not be an switch case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tinnedkarma Anything wrong? Are you sure that converting it to a switch-case is feasible or better?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is my reasoning:

  • a do while(0) loop is used, but the loop wont ever "loop", so it's a do? :D Then is the same as using naked {}, which will create a scope and execute the instructions once.
  • there is a set/fixed amount of log levels defined in the syslog.h, so it's arguably better to uses a switch case (look-up table) than "falling through" if-else.

I do see the same concept used throughout, so maybe there are other reasons that I may not know, hence why I addressed this as a question.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tinnedkarma

a do while(0) loop is used, but the loop wont ever "loop", so it's a do? :D Then is the same as using naked {}, which will create a scope and execute the instructions once.

Common usage; it can avoid syntax issues such as those occurring after expansion:

#define LOG_ERROR(msg) \
    fprintf(stderr, "Error: %s\n", msg); \
    exit(1);

if (error)
    LOG_ERROR(...);

/* After preprocessing */

if (error)
    fprintf(...); exit(1);

As for runtime efficiency, the compiler will perform optimizations.

there is a set/fixed amount of log levels defined in the syslog.h, so it's arguably better to uses a switch case (look-up table) than "falling through" if-else.

This requires listing all levels; moreover, some code inspection tools will consider that some cases are missing breaks.

switch (p)
  {
    case LOG_EMERG:
    case LOG_ALERT:
    case LOG_CRIT:
    case LOG_ERR:
        init_err(__VA_ARGS__);
        break;
    case LOG_WARNING:
        init_warn(__VA_ARGS__);
        break;
    /* ... ... */
  }

include/syslog.h:

#define LOG_EMERG     0  /* System is unusable */
#define LOG_ALERT     1  /* Action must be taken immediately */
#define LOG_CRIT      2  /* Critical conditions */
#define LOG_ERR       3  /* Error conditions */
#define LOG_WARNING   4  /* Warning conditions */
#define LOG_NOTICE    5  /* Normal, but significant, condition */
#define LOG_INFO      6  /* Informational message */
#define LOG_DEBUG     7  /* Debug-level message */

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Common usage; it can avoid syntax issues such as those occurring after expansion:

Yes but the same issues can be avoided by adding the curly braces.

As for runtime efficiency, the compiler will perform optimizations.

No, the compiler will not optimize the code in any way, is will generate predicable assembly.
Here is an example of code optimization apache/nuttx#17206.
Personally I would avoid intentionally offloading work to the compiler. It's our job to write predictable code.

This requires listing all levels; moreover, some code inspection tools will consider that some cases are missing breaks.

Yet, I still think it's a cleaner. :D

We are not getting anywhere. That is my personal view, it's fine if we do not agree, and anyway more input is always welcome.

Copy link
Contributor

@linguini1 linguini1 Oct 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The do-while pattern is actually quite common, there are some scenarios where regular curly braces don't have the same semantics. That's why do-while (0) is used almost always for multiline macros: https://stackoverflow.com/questions/1067226/c-multi-line-macro-do-while0-vs-scope-block

It does get compiler-optimized out and I think it's clear enough to keep instead of curly braces.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's settled then


init_action_add_event(&am, "boot");

boardctl(BOARDIOC_INIT, 0);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh man boardctl does make so much more sense with a proper init system. I always found it clunky to do that kind of calls from nsh.

now it's great.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True! It was like a chicken-egg thing :-)

Copy link

@7rx8 7rx8 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking forward to using this from the main branch. I believe it's ready for integration as is and remaining bits can be ironed out later.

Thank you so much for contributing this useful init system and managing its integration in a very great way.

}

c = malloc(sizeof(*c) + len);
if (!c)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (!c)
if (c == NULL)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, but I don't think this modification is necessary.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is more readable and also was compilers are become more restrictive, soon or later they can start raising warning about this kind of test

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@acassis ok, done

}

service = init_service_find_by_pid(sm, pid);
if (service)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (service)
if (service != NULL)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, but I don't think this modification is necessary.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, done

@cederom
Copy link
Contributor

cederom commented Oct 27, 2025

I still think it would be good to refactor init_* to nxinit_* or at least have self-explanatory nxinit_main in the configs since application is called nxinit. What do you think guys? :-)

CONFIG_INIT_ENTRYPOINT="nxinit_main"

This component (abbreviated as "NxInit") is specifically developed
for NuttX and intended for system initialization. While we have
adopted the Android Init Language among various syntax options,
this is a brand-new implementation—it is not a port or variant of
Android Init.

Refer to the implementation of Android Init Language, consists of five broad
classes of statements: Actions, Commands, Services, Options, and Imports.

Actions support two types of triggers: event and action. Action triggers also
support runtime triggering. Services support lifecycle management, including
automatic restart (at specified intervals), and starting/stopping
individually or by class. Import supports files or directories, and we may
add a static method in the future. The following are some differences:
  1. The Android Init Language treats lines starting with `#` as comments,
     while we use a preprocessor to handle comments.
  2. For action commands, we can omit "exec" and directly execute
     built-in apps or nsh builtins.
  3. Regarding the property service, users can either adapt it by self or
     directly use the preset NVS-based properties.
  4. Only part of standard action commands and service options are
     implemented currlently.

To enable system/nxinit:
  ```diff
  -CONFIG_INIT_ENTRYPOINT="nsh_main"
  +CONFIG_INIT_ENTRYPOINT="init_main"
  +CONFIG_SYSTEM_NXINIT=y
  ```

For format and additional details, refer to:
  https://android.googlesource.com/platform/system/core/+/
  master/init/README.md

Signed-off-by: wangjianyu3 <[email protected]>
Usage:
  class_start <classname>
  class_stop <classname>

Signed-off-by: wangjianyu3 <[email protected]>
Enable CONFIG_SYSTEM_SYSTEM to support nsh builtins, which depends on nsh.

Signed-off-by: wangjianyu3 <[email protected]>
If the command of an action takes too long (greater than
CONFIG_SYSTEM_NXINIT_ACTION_WARN_SLOW milliseconds, 50 ms by default),
a warning log will be output for analysis and debugging.

For example:

  a. sleep 1
       [    0.340000] [ 3] [ 0] init_main: executing NSH command 'sleep 1'
       [    1.360000] [ 3] [ 0] init_main: NSH command 'sleep 1' exited 0
     > [    1.360000] [ 3] [ 0] init_main: command 'sleep' took 1020 ms

  b. hello (add sleep(1) to examples/hello)

       [    1.390000] [ 3] [ 0] init_main: executed command 'hello' pid 14
       [    1.390000] [ 3] [ 0] init_main: waiting 'hello' pid 14
       Hello, World!!
     > [    2.400000] [ 3] [ 0] init_main: command 'hello' pid 14 took 1010 ms
       [    2.400000] [ 3] [ 0] init_main: command 'hello' pid 14 exited status 0

Signed-off-by: wangjianyu3 <[email protected]>
Add support for the oneshot option to the service.

Test
  - RC
      service telnet telnetd
          class test
    >     oneshot
          restart_period 3000
  - Runtime
      [    0.150000] [ 3] [ 0] init_main: == Dump Services ==
      ...
      [    0.160000] [ 3] [ 0] init_main: Service 0x40486aa8 name 'telnet' path 'telnetd'
      [    0.160000] [ 3] [ 0] init_main:   pid: 0
      [    0.160000] [ 3] [ 0] init_main:   arguments:
      [    0.160000] [ 3] [ 0] init_main:       [0] 'service'
      [    0.160000] [ 3] [ 0] init_main:       [1] 'telnet'
      [    0.160000] [ 3] [ 0] init_main:       [2] 'telnetd'
      [    0.160000] [ 3] [ 0] init_main:   classes:
      [    0.160000] [ 3] [ 0] init_main:     'test'
      [    0.170000] [ 3] [ 0] init_main:   restart_period: 3000
      [    0.170000] [ 3] [ 0] init_main:   reboot_on_failure: -1
      [    0.170000] [ 3] [ 0] init_main:   flags:
    > [    0.170000] [ 3] [ 0] init_main:     'oneshot'
      ...
      [    0.370000] [ 3] [ 0] init_main: starting service 'telnet' ...
      [    0.380000] [ 3] [ 0] init_main: service 'telnet' flag 0x2 add 0x4
      [    0.380000] [ 3] [ 0] init_main:   +flag 'running'
      [    0.380000] [ 3] [ 0] init_main: service 'telnet' flag 0x6 add 0x8
      [    0.380000] [ 3] [ 0] init_main:   -flag 'restarting'
      [    0.380000] [ 3] [ 0] init_main: service 'telnet' flag 0x6 add 0x1
      [    0.380000] [ 3] [ 0] init_main:   -flag 'disabled'
      [    0.380000] [ 3] [ 0] init_main: started service 'telnet' pid 9
      ...
      nsh> kill -9 9
      nsh> [    7.350000] [ 3] [ 0] init_main: service 'telnet' flag 0x6 add 0x4
      [    7.350000] [ 3] [ 0] init_main:   -flag 'running'
      [    7.350000] [ 3] [ 0] init_main: service 'telnet' flag 0x2 add 0x80000001
      [    7.350000] [ 3] [ 0] init_main:   +flag 'disabled'
      [    7.350000] [ 3] [ 0] init_main:   +flag 'remove'
      [    7.350000] [ 3] [ 0] init_main: service 'telnet' pid 9 exited status 1
    > [    7.360000] [ 3] [ 0] init_main: removing service 'telnet' ...

Signed-off-by: wangjianyu3 <[email protected]>
Format: exec_start <service>

Start the specified service and pause the processing of any additional
initialization commands until the service completes its execution. This
command operates similarly to the `exec` command; the key difference is
that it utilizes an existing service definition rather than requiring
the `exec` argument vector.

This feature is particularly intended for use with the `reboot_on_failure`
built-in command to perform all types of essential checks during system boot.

Signed-off-by: wangjianyu3 <[email protected]>
Add support for the reboot_on_failure option to the service.

When the execution of a command within a certain action fails (returning
a non-zero status code), NxInit will continue to execute subsequent commands or
actions and will not proactively terminate the startup process. To implement
the functionality of "terminating the startup process after a command
execution fails", there are two methods:
a. Execute conditional statements (if/then/else/fi) via exec command,
   but this depends on sh:
   ```
   on init
       exec -- set +e; \
               mount -t fatfs /dev/data /data ; \
               if [ $? -ne 0 ] ; \
               then \
                 echo "failed" ; \
                 reboot ; \
               else \
                 echo "succeed" ; \
               fi;
   ```
b. Via service's oneshot + reboot_on_failure:
   /* Although the example uses sh, it does not depend on it and can be
    * replaced with any other Builtin Apps.
    */
   ```
   on init
       exec_start mkdir_tmp
       ls /tmp

   service mkdir_tmp sh -c "mkdir /tmp"
       reboot_on_failure 0
       oneshot
   ```

Test
  - RC
      service console sh
          class core
          override
    >     reboot_on_failure 0
          restart_period 10000
  - Runtime
      [    0.150000] [ 3] [ 0] init_main: == Dump Services ==
      ...
      [    0.170000] [ 3] [ 0] init_main: Service 0x40486ea0 name 'console' path 'sh'
      [    0.170000] [ 3] [ 0] init_main:   pid: 0
      [    0.170000] [ 3] [ 0] init_main:   arguments:
      [    0.170000] [ 3] [ 0] init_main:       [0] 'service'
      [    0.170000] [ 3] [ 0] init_main:       [1] 'console'
      [    0.170000] [ 3] [ 0] init_main:       [2] 'sh'
      [    0.170000] [ 3] [ 0] init_main:   classes:
      [    0.170000] [ 3] [ 0] init_main:     'core'
      [    0.170000] [ 3] [ 0] init_main:   restart_period: 10000
    > [    0.170000] [ 3] [ 0] init_main:   reboot_on_failure: 0
      [    0.170000] [ 3] [ 0] init_main:   flags:
      [    0.170000] [ 3] [ 0] init_main:     'override'
      ...
      [    0.380000] [ 3] [ 0] init_main: started service 'console' pid 4
      ...
      nsh> kill -9 4
      [    8.060000] [ 3] [ 0] init_main: service 'console' flag 0x20000004 add 0x4
      [    8.060000] [ 3] [ 0] init_main:   -flag 'running'
      [    8.060000] [ 3] [ 0] init_main: service 'console' flag 0x20000000 add 0x8
      [    8.060000] [ 3] [ 0] init_main:   +flag 'restarting'
    > [    8.060000] [ 3] [ 0] init_main: Error reboot on failure of service 'console' reason 0

Signed-off-by: wangjianyu3 <[email protected]>
@JianyuWang0623
Copy link
Contributor Author

I still think it would be good to refactor init_* to nxinit_* or at least have self-explanatory nxinit_main in the configs since application is called nxinit. What do you think guys? :-)

CONFIG_INIT_ENTRYPOINT="nxinit_main"

@cederom Thank you. This is my view: whether it is the current NxInit, or SystemV Init (which may be added later), or other versions, from a functional perspective, the default value of PROGNAME should be "init". For example, in some Linux distributions, even systemd uses the widely recognized name "init":

$ cat /etc/lsb-release
DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=22.04
DISTRIB_CODENAME=jammy
DISTRIB_DESCRIPTION="Ubuntu 22.04.3 LTS"

$ ls /usr/sbin/init -l
lrwxrwxrwx 1 root root 20 Jun  4 22:17 /usr/sbin/init -> /lib/systemd/systemd
SYSTEMD(1)
NAME
       systemd, init - systemd system and service manager

If there is anything wrong with what I said, please feel free to correct me, thanks.

@7rx8
Copy link

7rx8 commented Oct 28, 2025

Hello

I do not agree with this closure. This is a great addition to nuttx that was worked on for a long time and is almost ready. Why are you closing this?

@JianyuWang0623 has worked a lot of time to reply to community requests there is no point in closing this.

@cederom
Copy link
Contributor

cederom commented Oct 28, 2025

@cederom: I still think it would be good to refactor init_* to nxinit_* or at least have self-explanatory nxinit_main in the configs since application is called nxinit. What do you think guys? :-)

CONFIG_INIT_ENTRYPOINT="nxinit_main"

@JianyuWang0623: @cederom Thank you. This is my view: whether it is the current NxInit, or SystemV Init (which may be added later), or other versions, from a functional perspective, the default value of PROGNAME should be "init". For example, in some Linux distributions, even systemd uses the widely recognized name "init":

$ cat /etc/lsb-release
DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=22.04
DISTRIB_CODENAME=jammy
DISTRIB_DESCRIPTION="Ubuntu 22.04.3 LTS"

$ ls /usr/sbin/init -l
lrwxrwxrwx 1 root root 20 Jun  4 22:17 /usr/sbin/init -> /lib/systemd/systemd
SYSTEMD(1)
NAME
       systemd, init - systemd system and service manager

If there is anything wrong with what I said, please feel free to correct me, thanks.

Okay @JianyuWang0623 as discussed in different PR now I get your point - there may be different init applications and all of them will provide the same init_main entry point, the kconfig file will use the same entry point for all of them, there will be no conflict because only one can be selected at time - this is more versatile approach thank you!! :-)

@cederom
Copy link
Contributor

cederom commented Oct 28, 2025

@xiaoxiang781216 we can reopen this and related PRs, we have an active discussion on the mailing list, this functionality is useful and welcome by the community :-)

GUYS PLEASE USE DEV@ MAILING LIST TO BE PART OF THE COMMUNITY AND ACTIVELY DISCUSS CHANGES AND PLANS WITH THE NUTTX COMMUNITY THIS IS OUR MAIN COMMUNICATION CHANNEL! :-)

@linguini1
Copy link
Contributor

Please consider reopening this PR; I was really looking forward to this functionality as it solves quite a number of issues I had with NuttX initialization.

@cederom
Copy link
Contributor

cederom commented Oct 28, 2025

Yup, this is a very nice starting point for desired and missing functionality in NuttX, that is purely optional and does not break anything for sure, big thank you and congratulations @JianyuWang0623!

Also thank you @JianyuWang0623 for great discussion here and on the dev@ mailing list this is really important so we work together as the community even if there are tons of questions and initial doubts all are solved now! :-)

NXINIT will for sure grow in future, having it merged will allow better testing and adaptation :-) When the PR is reopened it gets approval from my side (cannot approve closed PR) :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants