linguini1 commented on code in PR #3504: URL: https://github.com/apache/nuttx-apps/pull/3504#discussion_r3306862375
########## system/nxinit/Kconfig: ########## @@ -0,0 +1,125 @@ +# +# For a description of the syntax of this configuration file, +# see the file kconfig-language.txt in the NuttX tools repository. +# + +config SYSTEM_NXINIT + tristate "NuttX Init" + default n + depends on LIBC_EXECFUNCS + depends on SCHED_CHILD_STATUS + ---help--- + Enable NxInit(NuttX Init) component for system initialization. + The script of NxInit(init.rc) is compatible with the Android Init Language syntax. + +if SYSTEM_NXINIT + +# +# Basic Review Comment: These should maybe be `comment "Basic"`, etc. to show up in the menu. ########## system/nxinit/Kconfig: ########## @@ -0,0 +1,125 @@ +# +# For a description of the syntax of this configuration file, +# see the file kconfig-language.txt in the NuttX tools repository. +# + +config SYSTEM_NXINIT + tristate "NuttX Init" + default n + depends on LIBC_EXECFUNCS + depends on SCHED_CHILD_STATUS + ---help--- + Enable NxInit(NuttX Init) component for system initialization. + The script of NxInit(init.rc) is compatible with the Android Init Language syntax. + +if SYSTEM_NXINIT + +# +# Basic +# + +config SYSTEM_NXINIT_PRIORITY + int "Thread priority" + default 100 + +config SYSTEM_NXINIT_STACKSIZE + int "Stack size" + default DEFAULT_TASK_STACKSIZE + +config SYSTEM_NXINIT_PROGNAME + string "Program name" + default "init" + +# +# RC +# + +config SYSTEM_NXINIT_RC_LINE_MAX + int "Max line length of RC file" + default 128 + ---help--- + Maximum line length of RC file. + More details: https://android.googlesource.com/platform/system/core/+/master/init/README.md + +# +# Action +# + +config SYSTEM_NXINIT_ACTION_CMD_ARGS_MAX + int "Max number of command arguments" + default 8 + ---help--- + Maximum number of command arguments. + Form: + ``` + on <trigger> + <command> + <command> + <command> + ... + ``` + +config SYSTEM_NXINIT_ACTION_WARN_SLOW + int "Warn if command takes too long" Review Comment: `int "Slow command warning timeout"` (slightly clearer to user) ########## system/nxinit/service.h: ########## @@ -0,0 +1,144 @@ +/**************************************************************************** + * apps/system/nxinit/service.h + * + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. The + * ASF licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the + * License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + * + ****************************************************************************/ + +#ifndef __APPS_SYSTEM_NXINIT_SERVICE_H +#define __APPS_SYSTEM_NXINIT_SERVICE_H + +/**************************************************************************** + * Included Files + ****************************************************************************/ + +#include <nuttx/list.h> + +#include <stdint.h> +#include <sys/types.h> + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +/* Ref to Android Init + * URL: https://android.googlesource.com/platform/system/core/+/ + * android16-release/init/service.h#42 + */ +#define SVC_DISABLED (1 << 0) /* do not autostart with class */ +#define SVC_ONESHOT (1 << 1) /* do not restart on exit */ +#define SVC_RUNNING (1 << 2) /* currently active */ +#define SVC_RESTARTING (1 << 3) /* waiting to restart */ + +/* This service should be stopped with SIGTERM instead of SIGKILL. + * Will still be SIGKILLed after timeout period of 200 ms. + */ + +#define SVC_GENTLE_KILL (1 << 13) + +/* Flags below are new added. + */ + +/* Override the previous definition for a service with the same name */ + +#define SVC_OVERRIDE (1 << 29) + +/* Already sent SIGKILL, and should not send again. */ + +#define SVC_SIGKILL (1 << 30) + +/* Should be removed immediately after stopped */ + +#define SVC_REMOVE (1 << 31) + +/**************************************************************************** + * Public Types + ****************************************************************************/ + +struct service_class_s +{ + struct list_node node; /* Service class list node */ + FAR char name[0]; Review Comment: What is 0 size array? Just a `char *`? ########## system/nxinit/service.c: ########## @@ -0,0 +1,682 @@ +/**************************************************************************** + * apps/system/nxinit/service.c + * + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. The + * ASF licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the + * License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + * + ****************************************************************************/ + +/**************************************************************************** + * Included Files + ****************************************************************************/ + +#include <nuttx/clock.h> +#include <sys/boardctl.h> + +#include <assert.h> +#include <errno.h> +#include <string.h> +#include <stdlib.h> +#include <time.h> +#include <signal.h> +#include <spawn.h> +#include <sys/param.h> + +#include "init.h" +#include "parser.h" +#include "service.h" + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +#define SYSTEM_NXINIT_SERVICE_GENTLE_KILL_TIMEOUT 200 + +#define check_flags(s, f) ((s)->flags & (f)) + +#ifdef CONFIG_SYSTEM_NXINIT_DEBUG +# define dump_flags(fmt, flags) \ + do \ + { \ + size_t _i; \ + for (_i = 0; _i < nitems(g_flag_str); _i++) \ + { \ + if (g_flag_str[_i].flag & (flags)) \ + { \ + init_debug(fmt, g_flag_str[_i].str); \ + } \ + } \ + } \ + while(0) +#else +# define dump_flags(...) +#endif + +/**************************************************************************** + * Private Types + ****************************************************************************/ + +struct cmd_map_s +{ + FAR const char *cmd; + uint8_t minargs; + uint8_t maxargs; + int (*func)(FAR struct service_manager_s *, int, FAR char **); +}; + +#ifdef CONFIG_SYSTEM_NXINIT_DEBUG +struct flag_str_s +{ + uint32_t flag; + FAR const char *str; +}; +#endif + +/**************************************************************************** + * Private Function Prototypes + ****************************************************************************/ + +static int option_class(FAR struct service_manager_s *sm, + int argc, FAR char **argv); +static int option_gentle_kill(FAR struct service_manager_s *sm, + int argc, FAR char **argv); +static int option_restart_period(FAR struct service_manager_s *sm, + int argc, FAR char **argv); +static int option_override(FAR struct service_manager_s *sm, + int argc, FAR char **argv); +static int option_oneshot(FAR struct service_manager_s *sm, + int argc, FAR char **argv); +#ifdef CONFIG_BOARDCTL_RESET +static int option_reboot_on_failure(FAR struct service_manager_s *sm, + int argc, FAR char **argv); +#endif + +/**************************************************************************** + * Private Data + ****************************************************************************/ + +static const struct cmd_map_s g_option[] = +{ + {"class", 2, 99, option_class}, Review Comment: Should max arguments not be limited by `SYSTEM_NXINIT_ACTION_CMD_ARGS_MAX`? Or is that for something else? 99 seems high ########## system/nxinit/init.c: ########## @@ -0,0 +1,254 @@ +/**************************************************************************** + * apps/system/nxinit/init.c + * + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. The + * ASF licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the + * License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + * + ****************************************************************************/ + +/**************************************************************************** + * Included Files + ****************************************************************************/ + +#include <nuttx/config.h> + +#include <errno.h> +#include <poll.h> +#include <sys/param.h> +#include <sys/boardctl.h> +#include <sys/wait.h> + +#include <netutils/netinit.h> + +#include "action.h" +#include "builtin.h" +#include "init.h" +#include "import.h" +#include "service.h" + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +#define MS2TIMESPEC(ts, ms) \ + ((ms) == INT_MAX ? NULL \ + : ((ts)->tv_sec = (ms) / 1000, \ + (ts)->tv_nsec = ((ms) % 1000) * 1000000, (ts))) + +/**************************************************************************** + * Private Types + ****************************************************************************/ + +struct init_event_s +{ + FAR struct pollfd *pfd; + FAR void *priv; + FAR struct service_manager_s *sm; + FAR struct action_manager_s *am; + CODE int (*init) (FAR struct init_event_s *); + CODE void (*handle) (FAR struct init_event_s *); + CODE void (*deinit) (FAR struct init_event_s *); +}; + +/**************************************************************************** + * Private Functions + ****************************************************************************/ + +static void reap_process(FAR struct service_manager_s *sm, + FAR struct action_manager_s *am) +{ + FAR const char *name = "unknown"; + FAR const char *status; + FAR struct service_s *service; + int wstatus; + int ret; + int pid; + + for (; ; ) + { + pid = waitpid(-1, +#ifdef CONFIG_SYSTEM_NXINIT_DEBUG + &wstatus, +#else + NULL, +#endif + WNOHANG); + if (pid <= 0) + { + break; + } + else if (WIFEXITED(wstatus)) + { + status = "status"; + ret = WEXITSTATUS(wstatus); + } + else if (WIFSIGNALED(wtatus)) + { + status = "signal"; + ret = WTERMSIG(wstatus); + } + else + { + continue; + } + + if (pid == am->pid_running) + { + name = am->running->argv[0]; + init_action_reap_command(am); + } + + service = init_service_find_by_pid(sm, pid); + if (service != NULL) + { + name = service->argv[1]; + init_service_reap(service, ret); + } + + init_log(service ? LOG_WARNING : LOG_DEBUG, + "%s '%s' pid %d exited %s %d", + service ? "Service" : "Command", name, pid, status, ret); + } +} + +/**************************************************************************** + * Public Functions + ****************************************************************************/ + +int main(int argc, FAR char *argv[]) +{ + struct service_manager_s sm = + { + .services = LIST_INITIAL_VALUE(sm.services), + }; + + struct action_manager_s am = + { + .actions = LIST_INITIAL_VALUE(am.actions), + .ready_actions = LIST_INITIAL_VALUE(am.ready_actions), + .events = { 0 }, + .current = NULL, + .running = NULL, + .pid_running = -1, + .sm = &sm, + }; + + const struct parser_s parser[] = + { + {"import", init_import_parse, NULL, NULL}, + {"on", init_action_parse, NULL, &am}, + {"service", init_service_parse, init_service_check, &sm}, + {NULL}, + }; + + struct init_event_s ev[] = + { + }; + + struct pollfd pfds[nitems(ev)]; + size_t i; + int r; + +#ifdef CONFIG_USBDEV_TRACE + usbtrace_enable(TRACE_BITSET); +#endif + + for (i = 0; i < nitems(ev); i++) + { + ev[i].sm = &sm; + ev[i].am = &am; + ev[i].pfd = &pfds[i]; + r = ev[i].init(&ev[i]); + if (r < 0) + { + init_err("Init event %zu", i); + goto out; + } + } + + r = init_parse_config_file(parser, "/etc/init.d/init.rc"); Review Comment: I think `/etc/init.d/init.rc` should be a configurable location. Any way to bake init scripts into systems that don't want to add a whole file system? Maybe we can later add a build tool that compiles the script into a parse-able string instead? ########## system/nxinit/init.h: ########## @@ -0,0 +1,93 @@ +/**************************************************************************** + * apps/system/nxinit/init.h + * + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. The + * ASF licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the + * License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + * + ****************************************************************************/ + +#ifndef __APPS_SYSTEM_NXINIT_INIT_H +#define __APPS_SYSTEM_NXINIT_INIT_H + +/**************************************************************************** + * Included Files + ****************************************************************************/ + +#include <syslog.h> + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +#define TIMESPEC2MS(t) (((t).tv_sec * 1000) + (t).tv_nsec / 1000000) + +#ifdef CONFIG_SYSTEM_NXINIT_DEBUG +#define init_debug(...) syslog(LOG_DEBUG, ##__VA_ARGS__) +#define init_dump_args(argc, argv) \ + { \ + int _i; \ + for (_i = 0; _i < (argc); _i++) \ + { \ + init_debug(" argv[%d] '%s'", _i, (argv)[_i]); \ + } \ + } +#else +#define init_debug(...) +#define init_dump_args(...) +#endif + +#ifdef CONFIG_SYSTEM_NXINIT_INFO +#define init_info(...) syslog(LOG_INFO, ##__VA_ARGS__) +#else +#define init_info(...) +#endif + +#ifdef CONFIG_SYSTEM_NXINIT_WARN +#define init_warn(...) syslog(LOG_WARNING, ##__VA_ARGS__) +#else +#define init_warn(...) +#endif + +#ifdef CONFIG_SYSTEM_NXINIT_ERR +#define init_err(f, ...) syslog(LOG_ERR, "Error " f, ##__VA_ARGS__) +#else +#define init_err(...) +#endif + +#define init_log(p, ...) \ Review Comment: Why do we need this macro? We can just choose the appropriate other macro (`init_debug`, `init_err`, etc) for the log level. ########## system/nxinit/init.c: ########## @@ -0,0 +1,254 @@ +/**************************************************************************** + * apps/system/nxinit/init.c + * + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. The + * ASF licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the + * License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + * + ****************************************************************************/ + +/**************************************************************************** + * Included Files + ****************************************************************************/ + +#include <nuttx/config.h> + +#include <errno.h> +#include <poll.h> +#include <sys/param.h> +#include <sys/boardctl.h> +#include <sys/wait.h> + +#include <netutils/netinit.h> + +#include "action.h" +#include "builtin.h" +#include "init.h" +#include "import.h" +#include "service.h" + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +#define MS2TIMESPEC(ts, ms) \ + ((ms) == INT_MAX ? NULL \ + : ((ts)->tv_sec = (ms) / 1000, \ + (ts)->tv_nsec = ((ms) % 1000) * 1000000, (ts))) + +/**************************************************************************** + * Private Types + ****************************************************************************/ + +struct init_event_s +{ + FAR struct pollfd *pfd; + FAR void *priv; + FAR struct service_manager_s *sm; + FAR struct action_manager_s *am; + CODE int (*init) (FAR struct init_event_s *); + CODE void (*handle) (FAR struct init_event_s *); + CODE void (*deinit) (FAR struct init_event_s *); +}; + +/**************************************************************************** + * Private Functions + ****************************************************************************/ + +static void reap_process(FAR struct service_manager_s *sm, + FAR struct action_manager_s *am) +{ + FAR const char *name = "unknown"; + FAR const char *status; + FAR struct service_s *service; + int wstatus; + int ret; + int pid; + + for (; ; ) + { + pid = waitpid(-1, +#ifdef CONFIG_SYSTEM_NXINIT_DEBUG + &wstatus, +#else + NULL, +#endif + WNOHANG); + if (pid <= 0) + { + break; + } + else if (WIFEXITED(wstatus)) + { + status = "status"; + ret = WEXITSTATUS(wstatus); + } + else if (WIFSIGNALED(wtatus)) + { + status = "signal"; + ret = WTERMSIG(wstatus); + } + else + { + continue; + } + + if (pid == am->pid_running) + { + name = am->running->argv[0]; + init_action_reap_command(am); + } + + service = init_service_find_by_pid(sm, pid); + if (service != NULL) + { + name = service->argv[1]; + init_service_reap(service, ret); + } + + init_log(service ? LOG_WARNING : LOG_DEBUG, + "%s '%s' pid %d exited %s %d", + service ? "Service" : "Command", name, pid, status, ret); + } +} + +/**************************************************************************** + * Public Functions + ****************************************************************************/ + +int main(int argc, FAR char *argv[]) +{ + struct service_manager_s sm = + { + .services = LIST_INITIAL_VALUE(sm.services), + }; + + struct action_manager_s am = + { + .actions = LIST_INITIAL_VALUE(am.actions), + .ready_actions = LIST_INITIAL_VALUE(am.ready_actions), + .events = { 0 }, + .current = NULL, + .running = NULL, + .pid_running = -1, + .sm = &sm, + }; + + const struct parser_s parser[] = + { + {"import", init_import_parse, NULL, NULL}, + {"on", init_action_parse, NULL, &am}, + {"service", init_service_parse, init_service_check, &sm}, + {NULL}, + }; + + struct init_event_s ev[] = + { + }; + + struct pollfd pfds[nitems(ev)]; + size_t i; + int r; + +#ifdef CONFIG_USBDEV_TRACE + usbtrace_enable(TRACE_BITSET); +#endif + + for (i = 0; i < nitems(ev); i++) + { + ev[i].sm = &sm; + ev[i].am = &am; + ev[i].pfd = &pfds[i]; + r = ev[i].init(&ev[i]); + if (r < 0) + { + init_err("Init event %zu", i); + goto out; + } + } + + r = init_parse_config_file(parser, "/etc/init.d/init.rc"); + if (r < 0) + { + goto out; + } + + init_dump_actions(&am.actions); + init_dump_services(&sm.services); + + init_action_add_event(&am, "boot"); + + boardctl(BOARDIOC_INIT, 0); Review Comment: Deprecated. ########## system/nxinit/init.h: ########## @@ -0,0 +1,93 @@ +/**************************************************************************** + * apps/system/nxinit/init.h + * + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. The + * ASF licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the + * License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + * + ****************************************************************************/ + +#ifndef __APPS_SYSTEM_NXINIT_INIT_H +#define __APPS_SYSTEM_NXINIT_INIT_H + +/**************************************************************************** + * Included Files + ****************************************************************************/ + +#include <syslog.h> + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +#define TIMESPEC2MS(t) (((t).tv_sec * 1000) + (t).tv_nsec / 1000000) + +#ifdef CONFIG_SYSTEM_NXINIT_DEBUG +#define init_debug(...) syslog(LOG_DEBUG, ##__VA_ARGS__) Review Comment: For all logs, can we add `| LOG_USER` to make it clear this is a user-space log? ########## system/nxinit/init.h: ########## @@ -0,0 +1,93 @@ +/**************************************************************************** + * apps/system/nxinit/init.h + * + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. The + * ASF licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the + * License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + * + ****************************************************************************/ + +#ifndef __APPS_SYSTEM_NXINIT_INIT_H +#define __APPS_SYSTEM_NXINIT_INIT_H + +/**************************************************************************** + * Included Files + ****************************************************************************/ + +#include <syslog.h> + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +#define TIMESPEC2MS(t) (((t).tv_sec * 1000) + (t).tv_nsec / 1000000) + +#ifdef CONFIG_SYSTEM_NXINIT_DEBUG +#define init_debug(...) syslog(LOG_DEBUG, ##__VA_ARGS__) +#define init_dump_args(argc, argv) \ + { \ + int _i; \ + for (_i = 0; _i < (argc); _i++) \ + { \ + init_debug(" argv[%d] '%s'", _i, (argv)[_i]); \ + } \ + } +#else +#define init_debug(...) +#define init_dump_args(...) +#endif + +#ifdef CONFIG_SYSTEM_NXINIT_INFO +#define init_info(...) syslog(LOG_INFO, ##__VA_ARGS__) +#else +#define init_info(...) +#endif + +#ifdef CONFIG_SYSTEM_NXINIT_WARN +#define init_warn(...) syslog(LOG_WARNING, ##__VA_ARGS__) +#else +#define init_warn(...) +#endif + +#ifdef CONFIG_SYSTEM_NXINIT_ERR +#define init_err(f, ...) syslog(LOG_ERR, "Error " f, ##__VA_ARGS__) Review Comment: Maybe splitting hairs, but I don't think we need to append "Error". Syslog can already do this "[ERROR]" prefix if users want. This will save some amount of space on strings (maybe negligible but still). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
