From a860216097d6a169439027e0674351d9cf98c976 Mon Sep 17 00:00:00 2001 From: Davin McCall Date: Sun, 22 Sep 2024 16:48:12 +1000 Subject: [PATCH 01/51] dinit.8: List all user service description directories in FILES section --- doc/manpages/dinit.8.m4 | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/doc/manpages/dinit.8.m4 b/doc/manpages/dinit.8.m4 index 73722737..69054aae 100644 --- a/doc/manpages/dinit.8.m4 +++ b/doc/manpages/dinit.8.m4 @@ -41,12 +41,10 @@ See \fBSERVICE DESCRIPTION FILES\fR for details of the service description forma \fB\-d\fR \fIdir\fP, \fB\-\-services\-dir\fR \fIdir\fP Specifies \fIdir\fP as the directory containing service definition files. This can be specified multiple times for multiple service directories. -The default directories are not searched for services when this option is provided. -.sp -If not specified, the default for the user instance is \fI$XDG_CONFIG_HOME/dinit.d\fR, -\fI$HOME/.config/dinit.d\fR, \fI/etc/dinit.d/user\fR, \fI/usr/lib/dinit.d/user\fR and -\fI/usr/local/lib/dinit.d/user\fR or, for the system instance, each of \fI/etc/dinit.d\fR, \fI/run/dinit.d/\fR, -\fI/usr/local/lib/dinit.d\fR, and \fI/lib/dinit.d\fR (searched in that order). +.IP +The default service directories are listed in the \fBFILES\fR section. +Note that the default directories will not be searched when the \fB\-d\fR/\fB\-\-services\-dir\fR +option is specified. .TP \fB\-e\fR \fIfile\fP, \fB\-\-env\-file\fR \fIfile\fP Read initial environment from \fIfile\fP. @@ -324,7 +322,7 @@ value set previously as well as the effect of previous \fB!unset\fR and \fB!clea \fI/etc/dinit.d\fR, \fI/run/dinit.d\fR, \fI/usr/local/lib/dinit.d\fR, \fI/lib/dinit.d\fR Default locations for service description files. The directories are searched in the order listed. .TP -\fI$XDG_CONFIG_HOME/dinit.d\fR, \fI$HOME/.config/dinit.d\fR +\fI$XDG_CONFIG_HOME/dinit.d\fR, \fI$HOME/.config/dinit.d\fR, \fI/etc/dinit.d/user\fR, \fI/usr/lib/dinit.d/user\fR, \fI/usr/local/lib/dinit.d/user\fR Default location for service description files for user instances. The directories are searched in the order listed. .\" .SH SIGNALS From a08b365267364ee3730126f34af998b44c3358f8 Mon Sep 17 00:00:00 2001 From: Yao Zi Date: Sat, 28 Sep 2024 18:24:23 +0000 Subject: [PATCH 02/51] dinitcheck: warn about non-absolute executable path dinit's behavior depends on PATH environment if a service contains command with non-absolute executable path. dinitcheck may not even find correct executables in this case. Such services may lead to security problems, systemd has been searching executables only in compilation-time specified paths. As similar features do not exist in dinit and aren't very meaningful, we just warn about dangerous usage. References: https://www.man7.org/linux/man-pages/man5/systemd.service.5.html#COMMAND_LINES Signed-off-by: Yao Zi --- src/dinitcheck.cc | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/dinitcheck.cc b/src/dinitcheck.cc index a7efd576..824d6c5e 100644 --- a/src/dinitcheck.cc +++ b/src/dinitcheck.cc @@ -765,7 +765,11 @@ service_record *load_service(service_set_t &services, const std::string &name, auto check_command = [&](const char *setting_name, const char *command) { struct stat command_stat; - if (fstatat(dirfd, command, &command_stat, 0) == -1) { + if (command[0] != '/') { + report_service_description_err(name, + std::string("executable '") + command + "' is not an absolute path"); + } + else if (fstatat(dirfd, command, &command_stat, 0) == -1) { report_service_description_err(name, std::string("could not stat ") + setting_name + " executable '" + command + "': " + strerror(errno)); From 1aa0cdb9074fdab616fb4f6e56e873b96dc1731e Mon Sep 17 00:00:00 2001 From: Davin McCall Date: Sun, 29 Sep 2024 22:33:26 +1000 Subject: [PATCH 03/51] Fix restart of internal services --- src/control.cc | 22 ++++-- src/dinitctl.cc | 131 ++++++++++++++++++++++++------------ src/includes/control-cmds.h | 4 ++ src/includes/dinit-client.h | 86 +++++++++++++++++++++++ 4 files changed, 197 insertions(+), 46 deletions(-) diff --git a/src/control.cc b/src/control.cc index f5ee65b9..8a936085 100644 --- a/src/control.cc +++ b/src/control.cc @@ -306,7 +306,12 @@ bool control_conn_t::process_start_stop(cp_cmd pktType) } // 1 byte: packet type - // 1 byte: flags eg. pin in requested state (0 = no pin, 1 = pin) + // 1 byte: flags + // bit 0: 0 = no pin, 1 = pin + // bit 1: 0 = force stop, 1 = not forced ("gentle") + // bit 2: 0 = don't restart, 1 = restart after stopping + // --- (reserved) + // bit 7: 0 = no pre-ack, 1 = issue pre-ack // 4 bytes: service handle bool do_pin = ((rbuf[1] & 1) == 1); @@ -322,7 +327,16 @@ bool control_conn_t::process_start_stop(cp_cmd pktType) return true; } else { - char ack_buf[1] = { (char)cp_rply::ACK }; + char ack_buf[1] = { (char)cp_rply::PREACK }; + if (rbuf[1] & 128) { + // Issue PREACK before doing anything that might change service state (and cause a + // service event info packet to be issued as a result). This allows the client to + // determine whether the info packets were queued before or after the command was + // processed. + if (!queue_packet(ack_buf, 1)) return false; + } + + ack_buf[0] = (char)cp_rply::ACK; switch (pktType) { case cp_cmd::STARTSERVICE: @@ -383,7 +397,7 @@ bool control_conn_t::process_start_stop(cp_cmd pktType) wanted_state = service_state_t::STOPPED; } services->process_queues(); - if (service->get_state() == wanted_state && !do_restart) ack_buf[0] = (char)cp_rply::ALREADYSS; + if (service->get_state() == wanted_state) ack_buf[0] = (char)cp_rply::ALREADYSS; break; } case cp_cmd::WAKESERVICE: @@ -432,7 +446,7 @@ bool control_conn_t::process_start_stop(cp_cmd pktType) return false; } - if (! queue_packet(ack_buf, 1)) return false; + if (!queue_packet(ack_buf, 1)) return false; } clear_out: diff --git a/src/dinitctl.cc b/src/dinitctl.cc index 444ef3b7..8f5cd297 100644 --- a/src/dinitctl.cc +++ b/src/dinitctl.cc @@ -668,10 +668,6 @@ int main(int argc, char **argv) return 1; } -// Size of service status info (in various packets) -constexpr static unsigned STATUS_BUFFER_SIZE = 6 + ((sizeof(pid_t) > sizeof(int)) ? sizeof(pid_t) : sizeof(int)); -constexpr static unsigned STATUS_BUFFER5_SIZE = 6 + 2 * sizeof(int); - static const char * describe_state(bool stopped) { return stopped ? "stopped" : "started"; @@ -789,6 +785,40 @@ static void print_termination_details(int exit_si_code, int exit_si_status) } } +// Print reason for start failure. +static void print_failure_details(stopped_reason_t stop_reason, uint16_t launch_stage, + int exit_status, int exit_si_code, int exit_si_status) +{ + using namespace std; + + switch (stop_reason) { + case stopped_reason_t::DEPFAILED: + cout << "Reason: a dependency of the service failed to start. " + "Check dinit log.\n"; + break; + case stopped_reason_t::TIMEDOUT: + cout << "Reason: start timed out.\n"; + break; + case stopped_reason_t::EXECFAILED: + cout << "Reason: execution of service process failed:\n"; + cout << " Stage: " << exec_stage_descriptions[launch_stage] << "\n"; + cout << " Error: " << strerror(exit_status) << "\n"; + break; + case stopped_reason_t::FAILED: + cout << "Reason: service process terminated before ready: "; + if (exit_si_code != 0 || exit_si_status != 0) { + print_termination_details(exit_si_code, exit_si_status); + } + else { + print_termination_details(exit_status); + } + cout << "\n"; + break; + default: + cout << "Reason unknown/unrecognised. Check dinit log.\n"; + } +} + // Process a SERVICEEVENT[5] packet if it is related to the specified service handle, and // optionally report the service status to the user (verbose == true). The caller must ensure that // a complete packet of type SERVICEEVENT[5] is present in the buffer before calling. The size of @@ -849,46 +879,25 @@ static int process_service_event(cpbuffer_t &rbuffer, unsigned pktlen, handle_t if (verbose) { cout << "Service '" << service_name << "' failed to start.\n"; if (pktlen >= base_pkt_size + STATUS_BUFFER_SIZE) { + uint16_t launch_stage; + rbuffer.extract((char *)&launch_stage, base_pkt_size + 4, + sizeof(uint16_t)); + stopped_reason_t stop_reason = static_cast(rbuffer[base_pkt_size + 3]); int exit_status; int exit_si_code; - int exit_si_status = 0; + int exit_si_status; rbuffer.extract((char *)&exit_status, base_pkt_size + 6, sizeof(exit_status)); if (rbuffer[0] == (char)cp_info::SERVICEEVENT5) { + if (pktlen < base_pkt_size + STATUS_BUFFER5_SIZE) { + throw dinit_protocol_error(); + } exit_si_code = exit_status; - rbuffer.extract((char *)&exit_si_code, + rbuffer.extract((char *)&exit_si_status, base_pkt_size + 6 + sizeof(exit_si_code), sizeof(exit_si_status)); } - switch (stop_reason) { - case stopped_reason_t::DEPFAILED: - cout << "Reason: a dependency of the service failed to start. " - "Check dinit log.\n"; - break; - case stopped_reason_t::TIMEDOUT: - cout << "Reason: start timed out.\n"; - break; - case stopped_reason_t::EXECFAILED: - cout << "Reason: execution of service process failed:\n"; - uint16_t launch_stage; - rbuffer.extract((char *)&launch_stage, base_pkt_size + 4, - sizeof(uint16_t)); - cout << " Stage: " << exec_stage_descriptions[launch_stage] << "\n"; - cout << " Error: " << strerror(exit_status) << "\n"; - break; - case stopped_reason_t::FAILED: - cout << "Reason: service process terminated before ready: "; - if (rbuffer[0] == (char)cp_info::SERVICEEVENT5) { - print_termination_details(exit_si_code, exit_si_status); - } - else { - print_termination_details(exit_status); - } - cout << "\n"; - break; - default: - cout << "Reason unknown/unrecognised. Check dinit log.\n"; - } + print_failure_details(stop_reason, launch_stage, exit_status, exit_si_code, exit_si_status); } } rbuffer.consume(pktlen); @@ -981,6 +990,8 @@ static int start_stop_service(int socknum, cpbuffer_t &rbuffer, const char *serv pcommand = cp_cmd::STOPSERVICE; } + observed_states_t seen_states; + // Need to issue command (eg STOPSERVICE/STARTSERVICE) // We'll do this regardless of the current service state / target state, since issuing // start/stop also sets or clears the "explicitly started" flag on the service. @@ -990,7 +1001,7 @@ static int start_stop_service(int socknum, cpbuffer_t &rbuffer, const char *serv { char flags = (do_pin ? 1 : 0) | ((pcommand == cp_cmd::STOPSERVICE && !do_force) ? 2 : 0); if (command == ctl_cmd::RESTART_SERVICE) { - flags |= 4; + flags |= (4 | 128); // restart, pre-ack } auto m = membuf() @@ -999,14 +1010,33 @@ static int start_stop_service(int socknum, cpbuffer_t &rbuffer, const char *serv .append(handle); write_all_x(socknum, m); - wait_for_reply(rbuffer, socknum); + wait_for_reply(rbuffer, socknum, handle, &seen_states); cp_rply reply_pkt_h = (cp_rply)rbuffer[0]; rbuffer.consume(1); // consume header + + if (reply_pkt_h == cp_rply::PREACK) { + // We should consider state changes seen only after the PREACK (i.e. between the + // PREACK and the main reply): + seen_states.started = false; + seen_states.stopped = false; + seen_states.failed_start = false; + + // PREACK will be followed by a 2nd reply, get that now: + wait_for_reply(rbuffer, socknum, handle, &seen_states); + reply_pkt_h = (cp_rply)rbuffer[0]; + rbuffer.consume(1); + } + if (reply_pkt_h == cp_rply::ALREADYSS) { bool already = (state == wanted_state); if (verbose) { - cout << "Service " << (already ? "(already) " : "") - << describe_state(do_stop) << "." << endl; + if (command == ctl_cmd::RESTART_SERVICE) { + cout << "Service restarted.\n"; + } + else { + cout << "Service " << (already ? "(already) " : "") + << describe_state(do_stop) << ".\n"; + } } return 0; // success! } @@ -1063,13 +1093,13 @@ static int start_stop_service(int socknum, cpbuffer_t &rbuffer, const char *serv cerr << "dinitctl: cannot start/restart/wake service, shutdown is in progress.\n"; return 1; } - if (reply_pkt_h != cp_rply::ACK && reply_pkt_h != cp_rply::ALREADYSS) { + if (reply_pkt_h != cp_rply::ACK) { cerr << "dinitctl: protocol error." << endl; return 1; } } - if (! wait_for_service) { + if (!wait_for_service) { if (verbose) { cout << "Issued " << describe_verb(do_stop) << " command successfully for service '" << service_name << "'." << endl; @@ -1079,11 +1109,28 @@ static int start_stop_service(int socknum, cpbuffer_t &rbuffer, const char *serv if (command == ctl_cmd::RESTART_SERVICE) { // for restart we want to display both "stopped" and "started" statuses - if (wait_service_state(socknum, rbuffer, handle, service_name, true, verbose) != 0) { + if (seen_states.stopped) { + if (verbose) { + cout << "Service '" << service_name << "' stopped.\n"; + } + } + else if (wait_service_state(socknum, rbuffer, handle, service_name, true, verbose) != 0) { return EXIT_FAILURE; } } + if (seen_states.started) { + if (verbose) { + cout << "Service '" << service_name << "' started.\n"; + } + } + else if (seen_states.failed_start) { + if (verbose) { + cout << "Service '" << service_name << "' failed to start.\n"; + print_failure_details(seen_states.stop_reason, 0 /* not applicable */, + seen_states.exit_status, seen_states.exit_si_code, seen_states.exit_si_status); + } + } return wait_service_state(socknum, rbuffer, handle, service_name, do_stop, verbose); } diff --git a/src/includes/control-cmds.h b/src/includes/control-cmds.h index 0e10f265..bb299ee7 100644 --- a/src/includes/control-cmds.h +++ b/src/includes/control-cmds.h @@ -163,6 +163,10 @@ enum class cp_rply : dinit_cptypes::cp_rply_t { // Retrieve complete environment ALLENV = 78, + + // "Pre-acknowledgement". Issued before main reply after restart command + // (to avoid race condition for client tracking service status) + PREACK = 79, }; // Information (out-of-band): diff --git a/src/includes/dinit-client.h b/src/includes/dinit-client.h index 8bc01fc5..8f2eac77 100644 --- a/src/includes/dinit-client.h +++ b/src/includes/dinit-client.h @@ -67,6 +67,24 @@ class control_sock_conn_err : public general_error } }; +// Observed service states (started/stopped). +struct observed_states_t +{ + bool started = false; + bool stopped = false; + bool failed_start = false; + + // In case of failed start: + stopped_reason_t stop_reason = stopped_reason_t::NORMAL; + int exit_status = 0; + int exit_si_code = 0; + int exit_si_status = 0; +}; + +// Size of service status info (in various packets) +constexpr static unsigned STATUS_BUFFER_SIZE = 6 + ((sizeof(pid_t) > sizeof(int)) ? sizeof(pid_t) : sizeof(int)); +constexpr static unsigned STATUS_BUFFER5_SIZE = 6 + 2 * sizeof(int); + // static_membuf: a buffer of a fixed size (N) with one additional value (of type T). Don't use this // directly, construct via membuf. template class static_membuf @@ -190,6 +208,74 @@ inline void wait_for_reply(cpbuffer_t &rbuffer, int fd) } } +inline void wait_for_reply(cpbuffer_t &rbuffer, int fd, dinit_cptypes::handle_t handle, observed_states_t *seen_states) +{ + fill_buffer_to(rbuffer, fd, 1); + + while (rbuffer[0] >= 100) { + // Information packet; discard. + cp_info pkt_type = (cp_info) rbuffer[0]; + fill_buffer_to(rbuffer, fd, 2); + unsigned pktlen = (unsigned char) rbuffer[1]; + + rbuffer.consume(1); // Consume one byte so we'll read one byte of the next packet + fill_buffer_to(rbuffer, fd, pktlen); + + if (value(pkt_type).is_in(cp_info::SERVICEEVENT, cp_info::SERVICEEVENT5) + && seen_states != nullptr) { + + // earlier versions do not include status info, the size in that case is + // base_pkt_size: + constexpr unsigned base_pkt_size = 2 + sizeof(dinit_cptypes::handle_t) + 1; + + if (pktlen < base_pkt_size) { + throw dinit_protocol_error(); + } + + // Extract handle, check for match + dinit_cptypes::handle_t ev_handle; + rbuffer.extract((char *)&ev_handle, 1, sizeof(ev_handle)); + service_event_t event = static_cast(rbuffer[1 + sizeof(ev_handle)]); + + if (ev_handle == handle) { + if (event == service_event_t::STOPPED) { + seen_states->stopped = true; + } + if (event == service_event_t::STARTED) { + seen_states->started = true; + } + if (event == service_event_t::FAILEDSTART) { + stopped_reason_t stop_reason = + static_cast(rbuffer[base_pkt_size + 2]); + + seen_states->failed_start = true; + seen_states->stop_reason = stop_reason; + + int exit_status; + int exit_si_code; + int exit_si_status; + rbuffer.extract((char *)&exit_status, base_pkt_size + 5, sizeof(exit_status)); + if (pkt_type == cp_info::SERVICEEVENT5) { + if (pktlen < base_pkt_size + STATUS_BUFFER5_SIZE) { + throw dinit_protocol_error(); + } + exit_si_code = exit_status; + rbuffer.extract((char *)&exit_si_status, + base_pkt_size + 5 + sizeof(exit_si_code), sizeof(exit_si_status)); + + seen_states->exit_si_code = exit_si_code; + seen_states->exit_si_status = exit_si_status; + } + + seen_states->exit_status = exit_status; + } + } + } + + rbuffer.consume(pktlen - 1); + } +} + // Wait for an info packet. If any other reply packet comes, throw a cp_read_exception. inline void wait_for_info(cpbuffer_t &rbuffer, int fd) { From 8653bf71aee8015d87eb7ce14a5805015413f265 Mon Sep 17 00:00:00 2001 From: Davin McCall Date: Tue, 1 Oct 2024 07:57:29 +1000 Subject: [PATCH 04/51] Igr test: add test of restarting an internal service --- src/igr-tests/igr-runner.cc | 7 +++++++ src/igr-tests/restart/sd/boot | 1 + src/igr-tests/restart/sd/internal | 0 3 files changed, 8 insertions(+) create mode 100644 src/igr-tests/restart/sd/internal diff --git a/src/igr-tests/igr-runner.cc b/src/igr-tests/igr-runner.cc index 844a127e..c87731b0 100644 --- a/src/igr-tests/igr-runner.cc +++ b/src/igr-tests/igr-runner.cc @@ -310,6 +310,8 @@ void force_stop_test() void restart_test() { + // Restart both a process service and an internal service. + igr_test_setup setup("restart"); std::string output_file = setup.prep_output_file("basic-ran"); std::string socket_path = setup.prep_socket_path(); @@ -338,6 +340,11 @@ void restart_test() nanosleepx(0, 1000000000u / 10u); igr_assert_eq("ran\n", read_file_contents(output_file)); + + // "dinitctl restart internal" + dinitctl_p.start("restart", {"-p", socket_path, "restart", "internal"}); + dinitctl_p.wait_for_term({1, 0} /* max 1 second */); + igr_assert_eq("Service restarted.\n", dinitctl_p.get_stdout()); } void check_basic_test() diff --git a/src/igr-tests/restart/sd/boot b/src/igr-tests/restart/sd/boot index f7c6964f..805fb77c 100644 --- a/src/igr-tests/restart/sd/boot +++ b/src/igr-tests/restart/sd/boot @@ -1,2 +1,3 @@ type = internal waits-for = basic +waits-for = internal diff --git a/src/igr-tests/restart/sd/internal b/src/igr-tests/restart/sd/internal new file mode 100644 index 00000000..e69de29b From 07ce1a6169ec8285c9840c12a1cd003899b2d4c0 Mon Sep 17 00:00:00 2001 From: Davin McCall Date: Tue, 1 Oct 2024 17:42:57 +1000 Subject: [PATCH 05/51] Avoid use of uninitialised variable values --- src/dinitctl.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/dinitctl.cc b/src/dinitctl.cc index 8f5cd297..22250362 100644 --- a/src/dinitctl.cc +++ b/src/dinitctl.cc @@ -885,8 +885,8 @@ static int process_service_event(cpbuffer_t &rbuffer, unsigned pktlen, handle_t stopped_reason_t stop_reason = static_cast(rbuffer[base_pkt_size + 3]); int exit_status; - int exit_si_code; - int exit_si_status; + int exit_si_code = 0; + int exit_si_status = 0; rbuffer.extract((char *)&exit_status, base_pkt_size + 6, sizeof(exit_status)); if (rbuffer[0] == (char)cp_info::SERVICEEVENT5) { if (pktlen < base_pkt_size + STATUS_BUFFER5_SIZE) { From 344199fc642437492e0d9d3ecc47bc9335e75401 Mon Sep 17 00:00:00 2001 From: Davin McCall Date: Tue, 1 Oct 2024 18:32:23 +1000 Subject: [PATCH 06/51] control: Switch issue order of SERVICEEVENT/SERVICEEVENT5 packets This simplifies things for clients. --- src/control.cc | 33 ++++++----- src/tests/cptests/cptests.cc | 103 +++++++++++++++++++---------------- 2 files changed, 75 insertions(+), 61 deletions(-) diff --git a/src/control.cc b/src/control.cc index 8a936085..babbebdb 100644 --- a/src/control.cc +++ b/src/control.cc @@ -1425,32 +1425,39 @@ void control_conn_t::service_event(service_record *service, service_event_t even while (i != end) { uint32_t key = i->second; std::vector pkt; - constexpr int pktsize = 3 + sizeof(key) + STATUS_BUFFER_SIZE; - pkt.reserve(pktsize); - pkt.push_back((char)cp_info::SERVICEEVENT); - pkt.push_back(pktsize); + + // There are two types of service event packet: v5+, and the original. For backwards + // compatibility, we send both. The new packet is sent first since this simplifies + // things for the client (eg if waiting for an event, the client will receive the + // packet with the most information first). + + // packet type (byte) + packet length (byte) + event type (byte) + key + status buffer + constexpr int pktsize5 = 3 + sizeof(key) + STATUS_BUFFER5_SIZE; + pkt.reserve(pktsize5); + pkt.push_back((char)cp_info::SERVICEEVENT5); + pkt.push_back(pktsize5); char *p = (char *)&key; for (unsigned j = 0; j < sizeof(key); j++) { pkt.push_back(*p++); } pkt.push_back(static_cast(event)); - pkt.resize(pktsize); - fill_status_buffer(pkt.data() + 3 + sizeof(key), service); + pkt.resize(pktsize5); + fill_status_buffer5(pkt.data() + 3 + sizeof(key), service); queue_packet(std::move(pkt)); - // packet type (byte) + packet length (byte) + event type (byte) + key + status buffer pkt.clear(); - constexpr int pktsize5 = 3 + sizeof(key) + STATUS_BUFFER5_SIZE; - pkt.reserve(pktsize5); - pkt.push_back((char)cp_info::SERVICEEVENT5); - pkt.push_back(pktsize5); + + constexpr int pktsize = 3 + sizeof(key) + STATUS_BUFFER_SIZE; + pkt.reserve(pktsize); + pkt.push_back((char)cp_info::SERVICEEVENT); + pkt.push_back(pktsize); p = (char *)&key; for (unsigned j = 0; j < sizeof(key); j++) { pkt.push_back(*p++); } pkt.push_back(static_cast(event)); - pkt.resize(pktsize5); - fill_status_buffer5(pkt.data() + 3 + sizeof(key), service); + pkt.resize(pktsize); + fill_status_buffer(pkt.data() + 3 + sizeof(key), service); queue_packet(std::move(pkt)); ++i; diff --git a/src/tests/cptests/cptests.cc b/src/tests/cptests/cptests.cc index 67faca10..ce73d727 100644 --- a/src/tests/cptests/cptests.cc +++ b/src/tests/cptests/cptests.cc @@ -363,24 +363,25 @@ void cptest_startstop() bp_sys::extract_written_data(fd, wdata); assert(wdata.size() == 1 + 7 + STATUS_BUFFER_SIZE /* ACK reply + info packet */ + 7 + STATUS_BUFFER5_SIZE /* + v5 info packet */); - assert(wdata[0] == (char)cp_info::SERVICEEVENT); - // First info packet (original protocol): - // packetsize, key (handle), event - assert(wdata[1] == 7 + STATUS_BUFFER_SIZE); + // First info packet (v5 protocol): + assert(wdata[0] == (char)cp_info::SERVICEEVENT5); handle_t ip_h; - std::copy(wdata.data() + 2, wdata.data() + 2 + sizeof(ip_h), reinterpret_cast(&ip_h)); + std::copy(wdata.data() + 2, wdata.data() + 2 + sizeof(ip_h), + reinterpret_cast(&ip_h)); assert(ip_h == h); assert(wdata[6] == static_cast(service_event_t::STARTED)); - // 2nd info packet (v5 protocol): - unsigned idx = 7 + STATUS_BUFFER_SIZE; - assert(wdata[idx] == (char)cp_info::SERVICEEVENT5); - std::copy(wdata.data() + idx + 2, wdata.data() + idx + 2 + sizeof(ip_h), - reinterpret_cast(&ip_h)); + // 2nd info packet (original protocol): + unsigned idx = 7 + STATUS_BUFFER5_SIZE; + assert(wdata[idx] == (char)cp_info::SERVICEEVENT); + // packetsize, key (handle), event + assert(wdata[idx + 1] == 7 + STATUS_BUFFER_SIZE); + std::copy(wdata.data() + idx + 2, wdata.data() + idx + 2 + sizeof(ip_h), reinterpret_cast(&ip_h)); assert(ip_h == h); assert(wdata[idx + 6] == static_cast(service_event_t::STARTED)); + // Reply packet: constexpr unsigned reply_start = 7 + STATUS_BUFFER_SIZE + 7 + STATUS_BUFFER5_SIZE; // we get ALREADYSS since it started immediately: assert(wdata[reply_start] == (char)cp_rply::ALREADYSS); @@ -397,21 +398,21 @@ void cptest_startstop() bp_sys::extract_written_data(fd, wdata); assert(wdata.size() == 1 + 7 + STATUS_BUFFER_SIZE + 7 + STATUS_BUFFER5_SIZE); - // Original status packet: - assert(wdata[0] == (char)cp_info::SERVICEEVENT); - // packetsize, key (handle), event - assert(wdata[1] == 7 + STATUS_BUFFER_SIZE); + // v5 status packet: + assert(wdata[0] == (char)cp_info::SERVICEEVENT5); + // packet size, handle, event + assert(wdata[1] == 7 + STATUS_BUFFER5_SIZE); std::copy(wdata.data() + 2, wdata.data() + 2 + sizeof(ip_h), reinterpret_cast(&ip_h)); assert(ip_h == h); - assert(wdata[6] == static_cast(service_event_t::STOPPED)); + assert(wdata[idx + 6] == static_cast(service_event_t::STOPPED)); - // v5 status packet: - idx = 7 + STATUS_BUFFER_SIZE; - assert(wdata[idx] == (char)cp_info::SERVICEEVENT5); - std::copy(wdata.data() + idx + 2, wdata.data() + idx + 2 + sizeof(ip_h), - reinterpret_cast(&ip_h)); + // Original status packet: + idx = 7 + STATUS_BUFFER5_SIZE; + assert(wdata[idx] == (char)cp_info::SERVICEEVENT); + assert(wdata[idx + 1] == 7 + STATUS_BUFFER_SIZE); + std::copy(wdata.data() + 2, wdata.data() + 2 + sizeof(ip_h), reinterpret_cast(&ip_h)); assert(ip_h == h); - assert(wdata[idx + 6] == static_cast(service_event_t::STOPPED)); + assert(wdata[6] == static_cast(service_event_t::STOPPED)); // we get ALREADYSS since it stopped immediately: assert(wdata[reply_start] == (char)cp_rply::ALREADYSS); @@ -745,18 +746,20 @@ void cptest_enableservice() assert(wdata.size() == 1 + 7 + STATUS_BUFFER_SIZE + 7 + STATUS_BUFFER5_SIZE /* ACK reply + 2x info packet */); - // Original service event: - assert(wdata[0] == (char)cp_info::SERVICEEVENT); - // packetsize, key (handle), event - assert(wdata[1] == 7 + STATUS_BUFFER_SIZE); + // v5 service event: + assert(wdata[0] == (char)cp_info::SERVICEEVENT5); + // size, handle, event + assert(wdata[1] == 7 + STATUS_BUFFER5_SIZE); handle_t ip_h; std::copy(wdata.data() + 2, wdata.data() + 2 + sizeof(ip_h), reinterpret_cast(&ip_h)); assert(ip_h == h2); assert(wdata[6] == static_cast(service_event_t::STARTED)); - // v5 service event: - unsigned idx = 7 + STATUS_BUFFER_SIZE; - assert(wdata[idx] == (char)cp_info::SERVICEEVENT5); + // Original service event: + unsigned idx = 7 + STATUS_BUFFER5_SIZE; + assert(wdata[idx] == (char)cp_info::SERVICEEVENT); + // packetsize, key (handle), event + assert(wdata[idx + 1] == 7 + STATUS_BUFFER_SIZE); std::copy(wdata.data() + idx + 2, wdata.data() + idx + 2 + sizeof(ip_h), reinterpret_cast(&ip_h)); assert(ip_h == h2); @@ -830,23 +833,25 @@ void cptest_restart() // info packet (service stopped) x 2 + ACK: assert(wdata.size() == 7 + STATUS_BUFFER_SIZE + 7 + STATUS_BUFFER5_SIZE + 1); - // Original info packet: - assert(wdata[0] == (char)cp_info::SERVICEEVENT); - assert(wdata[1] == 7 + STATUS_BUFFER_SIZE); + // v5 info packet: + assert(wdata[0] == (char)cp_info::SERVICEEVENT5); + assert(wdata[1] == 7 + STATUS_BUFFER5_SIZE); handle_t ip_h; std::copy(wdata.data() + 2, wdata.data() + 2 + sizeof(ip_h), reinterpret_cast(&ip_h)); assert(ip_h == h); assert(wdata[6] == static_cast(service_event_t::STOPPED)); - // v5 info packet: - unsigned idx = 7 + STATUS_BUFFER_SIZE; - assert(wdata[idx] == (char)cp_info::SERVICEEVENT5); - std::copy(wdata.data() + idx + 2, wdata.data() + idx + 2 + sizeof(ip_h), reinterpret_cast(&ip_h)); + // Original info packet: + unsigned idx = 7 + STATUS_BUFFER5_SIZE; + assert(wdata[idx] == (char)cp_info::SERVICEEVENT); + assert(wdata[idx + 1] == 7 + STATUS_BUFFER_SIZE); + std::copy(wdata.data() + idx + 2, wdata.data() + idx + 2 + sizeof(ip_h), + reinterpret_cast(&ip_h)); assert(ip_h == h); assert(wdata[idx + 6] == static_cast(service_event_t::STOPPED)); // ACK: - idx += 7 + STATUS_BUFFER5_SIZE; + idx += 7 + STATUS_BUFFER_SIZE; assert(wdata[idx] == (char)cp_rply::ACK); sset.process_queues(); @@ -859,15 +864,16 @@ void cptest_restart() bp_sys::extract_written_data(fd, wdata); assert(wdata.size() == 7 + STATUS_BUFFER_SIZE + 7 + STATUS_BUFFER5_SIZE); /* info packets */ - assert(wdata[0] == (char)cp_info::SERVICEEVENT); - assert(wdata[1] == 7 + STATUS_BUFFER_SIZE); + + assert(wdata[0] == (char)cp_info::SERVICEEVENT5); + assert(wdata[1] == 7 + STATUS_BUFFER5_SIZE); std::copy(wdata.data() + 2, wdata.data() + 2 + sizeof(ip_h), reinterpret_cast(&ip_h)); assert(ip_h == h); assert(wdata[6] == static_cast(service_event_t::STARTED)); - idx = 7 + STATUS_BUFFER_SIZE; - assert(wdata[idx] == (char)cp_info::SERVICEEVENT5); - assert(wdata[idx + 1] == 7 + STATUS_BUFFER5_SIZE); + idx = 7 + STATUS_BUFFER5_SIZE; + assert(wdata[idx] == (char)cp_info::SERVICEEVENT); + assert(wdata[idx + 1] == 7 + STATUS_BUFFER_SIZE); std::copy(wdata.data() + idx + 2, wdata.data() + idx + 2 + sizeof(ip_h), reinterpret_cast(&ip_h)); assert(ip_h == h); @@ -917,24 +923,25 @@ void cptest_wake() // ACK + 2 x info packet assert(wdata.size() == 1 + 7 + STATUS_BUFFER_SIZE + 7 + STATUS_BUFFER5_SIZE); - // Original info packet: - assert(wdata[0] == (char)cp_info::SERVICEEVENT); - assert(wdata[1] == 7 + STATUS_BUFFER_SIZE); + // v5 info packet: + assert(wdata[0] == (char)cp_info::SERVICEEVENT5); + assert(wdata[1] == 7 + STATUS_BUFFER5_SIZE); handle_t ip_h; std::copy(wdata.data() + 2, wdata.data() + 2 + sizeof(ip_h), reinterpret_cast(&ip_h)); assert(ip_h == h1); assert(wdata[6] == static_cast(service_event_t::STARTED)); - // v5 info packet: - unsigned idx = 7 + STATUS_BUFFER_SIZE; - assert(wdata[idx] == (char)cp_info::SERVICEEVENT5); + // Original info packet: + unsigned idx = 7 + STATUS_BUFFER5_SIZE; + assert(wdata[idx] == (char)cp_info::SERVICEEVENT); + assert(wdata[idx + 1] == 7 + STATUS_BUFFER_SIZE); std::copy(wdata.data() + idx + 2, wdata.data() + idx + 2 + sizeof(ip_h), reinterpret_cast(&ip_h)); assert(ip_h == h1); assert(wdata[idx + 6] == static_cast(service_event_t::STARTED)); // and then the ack (already started): - idx += 7 + STATUS_BUFFER5_SIZE; + idx += 7 + STATUS_BUFFER_SIZE; assert(wdata[idx] == (char)cp_rply::ALREADYSS); // now stop s2 (and therefore s1): From f1616ce1a039b2ee0de4c63a88963f38f1aa46fc Mon Sep 17 00:00:00 2001 From: Davin McCall Date: Tue, 1 Oct 2024 19:27:34 +1000 Subject: [PATCH 07/51] Remove redudnant packet size check --- src/dinitctl.cc | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/dinitctl.cc b/src/dinitctl.cc index 22250362..36589da6 100644 --- a/src/dinitctl.cc +++ b/src/dinitctl.cc @@ -889,9 +889,6 @@ static int process_service_event(cpbuffer_t &rbuffer, unsigned pktlen, handle_t int exit_si_status = 0; rbuffer.extract((char *)&exit_status, base_pkt_size + 6, sizeof(exit_status)); if (rbuffer[0] == (char)cp_info::SERVICEEVENT5) { - if (pktlen < base_pkt_size + STATUS_BUFFER5_SIZE) { - throw dinit_protocol_error(); - } exit_si_code = exit_status; rbuffer.extract((char *)&exit_si_status, base_pkt_size + 6 + sizeof(exit_si_code), sizeof(exit_si_status)); From fbde19c728ecf061b93829e24d83e4df4a02c32b Mon Sep 17 00:00:00 2001 From: Davin McCall Date: Tue, 1 Oct 2024 19:29:33 +1000 Subject: [PATCH 08/51] dinitctl: wait for service state via v5 or original service event Respond to both SERVICEEVENT5 (protocol v5) or original SERVICEEVENT when waiting for a service to reach a started/stopped state. The v5 info packet should arrive first (assuming new dinit daemon) and has more information, so will display better status info. --- src/dinitctl.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dinitctl.cc b/src/dinitctl.cc index 36589da6..56086141 100644 --- a/src/dinitctl.cc +++ b/src/dinitctl.cc @@ -917,7 +917,7 @@ static int wait_service_state(int socknum, cpbuffer_t &rbuffer, handle_t handle, unsigned pktlen = (unsigned char) rbuffer[1]; fill_buffer_to(rbuffer, socknum, pktlen); - if (rbuffer[0] == (char)cp_info::SERVICEEVENT) { + if (value((cp_info)rbuffer[0]).is_in(cp_info::SERVICEEVENT, cp_info::SERVICEEVENT5)) { int ret = process_service_event(rbuffer, pktlen, handle, service_name, do_stop, verbose); if (ret >= 0) { return ret; From 23a63773dc78beed7701ce81cf53e4f9590e80ba Mon Sep 17 00:00:00 2001 From: Davin McCall Date: Tue, 1 Oct 2024 20:21:05 +1000 Subject: [PATCH 09/51] Log a warning if not able to supervise bgprocess If we can't supervise a bgprocess (which should only be because the PID is not actually a child of the dinit process), log a warning. --- src/proc-service.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/proc-service.cc b/src/proc-service.cc index 57eb6832..da8fea61 100644 --- a/src/proc-service.cc +++ b/src/proc-service.cc @@ -737,6 +737,7 @@ bgproc_service::read_pid_file(proc_status_t *exit_status) noexcept if (waitid_r == -1 && errno == ECHILD) { // We can't track this child - check process exists: if (bp_sys::kill(pid, 0) == 0 || errno != ESRCH) { + log(loglevel_t::WARN, get_name(), ": unable to supervise process with pid ", pid); tracking_child = false; return pid_result_t::OK; } From ec94796ed5a813ccdd59ea59dab688d7e05f50c3 Mon Sep 17 00:00:00 2001 From: Davin McCall Date: Tue, 1 Oct 2024 22:18:03 +1000 Subject: [PATCH 10/51] fix: 2nd restart of bgprocess service hangs Because stop_issued is not reset when a bgprocess service stops, the next time we try to stop the service we don't actually signal it. This causes a hang (including at shutdown). --- src/proc-service.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/proc-service.cc b/src/proc-service.cc index da8fea61..96e4c06e 100644 --- a/src/proc-service.cc +++ b/src/proc-service.cc @@ -550,6 +550,7 @@ void bgproc_service::handle_exit_status() noexcept // We won't log a non-zero exit status or termination due to signal here - // we assume that the process died because we signalled it. if (stop_pid == -1 && !waiting_for_execstat) { + stop_issued = false; stopped(); } } From 24184e7a0c0988359c64c6329270dea8961934ae Mon Sep 17 00:00:00 2001 From: Davin McCall Date: Tue, 1 Oct 2024 22:40:10 +1000 Subject: [PATCH 11/51] Test: multiple start/stop for bgprocess --- src/tests/proctests.cc | 68 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/src/tests/proctests.cc b/src/tests/proctests.cc index 59252b78..526126cd 100644 --- a/src/tests/proctests.cc +++ b/src/tests/proctests.cc @@ -1520,6 +1520,73 @@ void test_bgproc_start_fail_pid() sset.remove_service(&p); } +void test_bgproc_restart_x2() +{ + using namespace std; + + service_set sset; + + ha_string command = "test-command"; + list> command_offsets; + command_offsets.emplace_back(0, command.length()); + std::list depends; + + bgproc_service p {&sset, "testproc", std::move(command), command_offsets, depends}; + init_service_defaults(p); + p.set_pid_file("/run/daemon.pid"); + sset.add_service(&p); + + p.start(); + sset.process_queues(); + + assert(p.get_state() == service_state_t::STARTING); + + base_process_service_test::exec_succeeded(&p); + sset.process_queues(); + + assert(p.get_state() == service_state_t::STARTING); + + supply_pid_contents("/run/daemon.pid"); + + base_process_service_test::handle_exit(&p, 0); + + assert(p.get_state() == service_state_t::STARTED); + assert(event_loop.active_timers.size() == 0); + + bp_sys::last_sig_sent = 0; + + p.stop(); + sset.process_queues(); + + assert(p.get_state() == service_state_t::STOPPING); + assert(bp_sys::last_sig_sent == SIGTERM); + + base_process_service_test::handle_exit(&p, 0); + assert(p.get_state() == service_state_t::STOPPED); + + // Start again: + p.start(); + sset.process_queues(); + assert(p.get_state() == service_state_t::STARTING); + base_process_service_test::exec_succeeded(&p); + sset.process_queues(); + assert(p.get_state() == service_state_t::STARTING); + supply_pid_contents("/run/daemon.pid"); + base_process_service_test::handle_exit(&p, 0); + assert(p.get_state() == service_state_t::STARTED); + + // Stop again: + bp_sys::last_sig_sent = 0; + p.stop(); + sset.process_queues(); + assert(p.get_state() == service_state_t::STOPPING); + assert(bp_sys::last_sig_sent == SIGTERM); + base_process_service_test::handle_exit(&p, 0); + assert(p.get_state() == service_state_t::STOPPED); + + sset.remove_service(&p); +} + void test_bgproc_unexpected_term() { using namespace std; @@ -2616,6 +2683,7 @@ int main(int argc, char **argv) RUN_TEST(test_bgproc_start, " "); RUN_TEST(test_bgproc_start_fail, " "); RUN_TEST(test_bgproc_start_fail_pid, " "); + RUN_TEST(test_bgproc_restart_x2, " "); RUN_TEST(test_bgproc_unexpected_term, ""); RUN_TEST(test_bgproc_smooth_recover, " "); RUN_TEST(test_bgproc_smooth_recove2, " "); From 8ca70d4ee30d2705bcd161655a780adfd19df638 Mon Sep 17 00:00:00 2001 From: q66 Date: Wed, 2 Oct 2024 14:50:07 +0200 Subject: [PATCH 12/51] Fix depends-ms.d reading --- src/includes/load-service.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/includes/load-service.h b/src/includes/load-service.h index 19348f8b..ff31c93a 100644 --- a/src/includes/load-service.h +++ b/src/includes/load-service.h @@ -1556,9 +1556,8 @@ void process_service_line(settings_wrapper &settings, const char *name, string & } case setting_id_t::DEPENDS_MS_D: { - string dependency_name = read_setting_value(input_pos, i, end); - settings.depends.emplace_back(load_service(dependency_name.c_str()), - dependency_type::MILESTONE); + string depends_ms_d = read_setting_value(input_pos, i, end); + process_dep_dir(settings.depends, depends_ms_d, dependency_type::MILESTONE); break; } case setting_id_t::AFTER: From 7bfcf5a01aa11a31ef13eed5a1ed0aec0fa4ab33 Mon Sep 17 00:00:00 2001 From: q66 Date: Tue, 8 Oct 2024 23:27:33 +0200 Subject: [PATCH 13/51] Fix status filling for new protocol --- src/control.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/control.cc b/src/control.cc index babbebdb..14187bf9 100644 --- a/src/control.cc +++ b/src/control.cc @@ -754,7 +754,7 @@ bool control_conn_t::list_services5() pkt_buf[0] = (char)cp_rply::SVCINFO; pkt_buf[1] = nameLen; - fill_status_buffer(&pkt_buf[2], sptr); + fill_status_buffer5(&pkt_buf[2], sptr); for (int i = 0; i < nameLen; i++) { pkt_buf[hdrsize+i] = name[i]; @@ -834,7 +834,7 @@ bool control_conn_t::process_service_status5() std::vector pkt_buf(2 + STATUS_BUFFER5_SIZE); pkt_buf[0] = (char)cp_rply::SERVICESTATUS; pkt_buf[1] = 0; - fill_status_buffer(pkt_buf.data() + 2, service); + fill_status_buffer5(pkt_buf.data() + 2, service); return queue_packet(std::move(pkt_buf)); } From a425ea62cd8a59112fc48f4517b62b7676aa4eef Mon Sep 17 00:00:00 2001 From: q66 Date: Tue, 8 Oct 2024 23:32:28 +0200 Subject: [PATCH 14/51] Fix accidental use of variable-length array --- src/dinitctl.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dinitctl.cc b/src/dinitctl.cc index 56086141..45f60da5 100644 --- a/src/dinitctl.cc +++ b/src/dinitctl.cc @@ -2378,7 +2378,7 @@ static int cat_service_log(int socknum, cpbuffer_t &rbuffer, const char *service cout << flush; bool trailing_nl = false; - char output_buf[rbuffer.get_size()]; + char output_buf[cpbuffer_t::get_size()]; while (bufsize > 0) { unsigned l = rbuffer.get_length(); if (l == 0) { From c5c307b6109626360abc863e0137438fe4dd8754 Mon Sep 17 00:00:00 2001 From: q66 Date: Wed, 9 Oct 2024 02:20:12 +0200 Subject: [PATCH 15/51] Increment current protocol version --- src/control.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/control.cc b/src/control.cc index 14187bf9..704cee21 100644 --- a/src/control.cc +++ b/src/control.cc @@ -18,7 +18,7 @@ using namespace dinit_cptypes; namespace { // Control protocol minimum compatible version and current version: constexpr uint16_t min_compat_version = 1; - constexpr uint16_t cp_version = 4; + constexpr uint16_t cp_version = 5; // check for value in a set template From 2d21fe862432ac8cc2bffb20b3ad248f04ea2787 Mon Sep 17 00:00:00 2001 From: Davin McCall Date: Mon, 14 Oct 2024 18:00:48 +1000 Subject: [PATCH 16/51] Document protocol v5 as released in 0.19.1 --- src/includes/control-cmds.h | 2 +- src/shutdown.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/includes/control-cmds.h b/src/includes/control-cmds.h index bb299ee7..cd9410c9 100644 --- a/src/includes/control-cmds.h +++ b/src/includes/control-cmds.h @@ -10,7 +10,7 @@ // 2 - dinit 0.17 (adds SETTRIGGER, CATLOG, SIGNAL) // 3 - dinit 0.17.1 (adds QUERYSERVICEDSCDIR) // 4 - dinit 0.18.0 (adds CLOSEHANDLE, GETALLENV) -// 5 - (unreleased) (process status now represented as ([int]si_code + [int]si_status) rather than +// 5 - dinit 0.19.1 (process status now represented as ([int]si_code + [int]si_status) rather than // a single integer; SERVICEEVENT5 sent alongside SERVICEEVENT) // Requests: diff --git a/src/shutdown.cc b/src/shutdown.cc index c9a13ab1..2e4ea915 100644 --- a/src/shutdown.cc +++ b/src/shutdown.cc @@ -30,7 +30,7 @@ // This utility communicates with the dinit daemon via a unix socket (specified in SYSCONTROLSOCKET). static constexpr uint16_t min_cp_version = 1; -static constexpr uint16_t max_cp_version = 1; +static constexpr uint16_t max_cp_version = 5; static constexpr auto reboot_execname = cts::literal(SHUTDOWN_PREFIX) + cts::literal("reboot"); static constexpr auto soft_reboot_execname = cts::literal(SHUTDOWN_PREFIX) + cts::literal("soft-reboot"); From e636abf7d93b227787c5a6fd7d973a5f0e1d29f7 Mon Sep 17 00:00:00 2001 From: Davin McCall Date: Mon, 14 Oct 2024 18:52:49 +1000 Subject: [PATCH 17/51] Version 0.19.1 --- NEWS | 17 +++++++++++++++++ README.md | 2 +- build/version.conf | 4 ++-- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/NEWS b/NEWS index 30b8d6d6..228aed11 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,20 @@ +== Version 0.19.1 (beta release #5) + +This is a fifth beta release, containing important bugfixes compared to the prior release. + +Thanks again to current sponsors: Paweł Zmarzły (pzmarzly), Wesley Moore (wezm), +M. Herdiansyah (konimex), q66, saolof, and private sponsors. + +With an influx of new features being contributed, a follow-up release is planned. Dinit 0.19.0 was +not the final beta after all, and neither will be 0.19.1. + +Fixes: + * Resolve issue causing service restarts to hang (affected bgprocess and internal services). + * Fix handling of "depends-ms.d" service setting (q66). + * Bump protocol version reported by dinit so that full exit status of processes is actually + reported (on OSes that support it) (q66). + * dinitcheck warns about non-absolute executable paths in service descriptions (Yao Zi). + == Version 0.19.0 (beta release #4) This is a fourth beta release, with bugfixes and some significant feature additions compared to diff --git a/README.md b/README.md index 610e35e2..fe18fd78 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # Dinit -v0.19.0 (beta) +v0.19.1 --- ![Dinit logo](doc/dinit-logo.png) diff --git a/build/version.conf b/build/version.conf index 285ceac3..934f1d3a 100644 --- a/build/version.conf +++ b/build/version.conf @@ -1,4 +1,4 @@ # Included from Makefiles. -VERSION=0.19.0 -MONTH=September +VERSION=0.19.1 +MONTH=October YEAR=2024 From 4949529880aabb64797028b633d01e2a4dbfd9e0 Mon Sep 17 00:00:00 2001 From: Jami Kettunen Date: Sun, 20 Oct 2024 22:53:29 +0300 Subject: [PATCH 18/51] Fix bad formatting of line number when a service fails to load It was adding "line X" directly after the service file path without any kind of separation. --- CONTRIBUTORS | 1 + src/includes/service.h | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTORS b/CONTRIBUTORS index 797debea..ff3dd217 100644 --- a/CONTRIBUTORS +++ b/CONTRIBUTORS @@ -12,6 +12,7 @@ The following people (in alphabetical order) have contributed: * Edd Barrett - Testing, documentation. * Fabien Poussin - Code, services, documentation. * James Knippes - Code + * Jami Kettunen - Code * Mobin Aydinfar - Code, documentation, CI * Oliver Amann - Code, testing, documentation * Locria Cyber - Code, documentation diff --git a/src/includes/service.h b/src/includes/service.h index 6de8b50d..8e25c493 100644 --- a/src/includes/service.h +++ b/src/includes/service.h @@ -223,12 +223,12 @@ inline void log_service_load_failure(service_description_exc &exc) if (exc.input_pos.get_line_num() != (unsigned)-1) { if (exc.setting_name == nullptr) { log(loglevel_t::ERROR, "Error in service description for '", exc.service_name, - "' (file ", exc.input_pos.get_file_name(), "line ", exc.input_pos.get_line_num(), "): ", + "' (file ", exc.input_pos.get_file_name(), ", line ", exc.input_pos.get_line_num(), "): ", exc.exc_description); } else { log(loglevel_t::ERROR, "Error in service description for '", exc.service_name, "': setting '", exc.setting_name, "' " - "(file ", exc.input_pos.get_file_name(), "line ", exc.input_pos.get_line_num(), "): ", + "(file ", exc.input_pos.get_file_name(), ", line ", exc.input_pos.get_line_num(), "): ", exc.exc_description); } } From 3aca3b1ba6f7be48ce9a57bdf29efb3ea2495fc9 Mon Sep 17 00:00:00 2001 From: Davin McCall Date: Tue, 22 Oct 2024 21:27:56 +1000 Subject: [PATCH 19/51] Fix example recovery service to have restart=false --- doc/linux/DINIT-AS-INIT.md | 6 ++++-- doc/linux/services/recovery | 1 + 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/doc/linux/DINIT-AS-INIT.md b/doc/linux/DINIT-AS-INIT.md index 3f03c471..48df8809 100644 --- a/doc/linux/DINIT-AS-INIT.md +++ b/doc/linux/DINIT-AS-INIT.md @@ -292,9 +292,11 @@ end, we have: There are two additional services, which are not depended on by any other service, and so do not normally start at all: -- `recovery` - this service is started by Dinit if boot fails (and if the user when prompted +- `recovery` - this service is started by Dinit if boot fails (and if the user, when prompted, then chooses the recovery option). It prompts for the root password and then provides a - shell. + shell. It has `restart = false` because once the administrator has finished repairing the system + configuration, they will exit the recovery shell, and at that point they should be offered the + various boot failure options (reboot etc) again. - `single` - this is a "single user mode" startup service which simply runs a shell. An unprivileged user cannot normally start this; doing so requires putting "single" on the kernel command line. When the shell exits, the `chain-to` setting will cause normal diff --git a/doc/linux/services/recovery b/doc/linux/services/recovery index ee9cee36..801f67b3 100644 --- a/doc/linux/services/recovery +++ b/doc/linux/services/recovery @@ -5,3 +5,4 @@ type = process command = /sbin/sulogin options = runs-on-console +restart = false From 664b29225a477e521f0e3bc91060a15ab2e52b3a Mon Sep 17 00:00:00 2001 From: q66 Date: Sun, 24 Nov 2024 18:42:17 +0100 Subject: [PATCH 20/51] Fix SVCDSCDIR replies on 64-bit big endian architectures While on LE this works, on BE it copies the other four bytes into the uint32_t, resulting in a zero. This in turn results in a protocol error on those architectures. --- src/control.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/control.cc b/src/control.cc index 704cee21..76d4a0e4 100644 --- a/src/control.cc +++ b/src/control.cc @@ -1296,7 +1296,7 @@ bool control_conn_t::process_query_dsc_dir() // 4 bytes (uint32_t) = directory length (no nul terminator) // N bytes = directory (no nul) std::vector reppkt; - size_t sdir_len = strlen(service->get_service_dsc_dir()); + auto sdir_len = static_cast(strlen(service->get_service_dsc_dir())); reppkt.resize(1 + sizeof(uint32_t) + sdir_len); // packet type, dir length, dir reppkt[0] = (char)cp_rply::SVCDSCDIR; std::memcpy(&reppkt[1], &sdir_len, sizeof(sdir_len)); From d333d41bce06abd746404a93184596a2fb8192b0 Mon Sep 17 00:00:00 2001 From: Davin McCall Date: Thu, 12 Dec 2024 19:52:55 +1000 Subject: [PATCH 21/51] Resolve dep dirs (.d) relative to the path of file containing setting If a file A is @included from a service description file B, and A has a "waits-for.d" setting (for example) with a non-absolute directory name, resolve the name relative to A rather than to B. Also fix a bug in dinitcheck that made it fail to find the dependency directory. --- src/dinitcheck.cc | 1 + src/load-service.cc | 10 ++++++---- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/dinitcheck.cc b/src/dinitcheck.cc index 824d6c5e..577633d7 100644 --- a/src/dinitcheck.cc +++ b/src/dinitcheck.cc @@ -663,6 +663,7 @@ service_record *load_service(service_set_t &services, const std::string &name, auto process_dep_dir_n = [&](std::list &deplist, const std::string &waitsford, dependency_type dep_type) -> void { + const string &service_filename = input_stack.current_file_name(); process_dep_dir(name.c_str(), service_filename, deplist, waitsford, dep_type); }; diff --git a/src/load-service.cc b/src/load-service.cc index b1f48f8d..7737a1c0 100644 --- a/src/load-service.cc +++ b/src/load-service.cc @@ -487,16 +487,18 @@ service_record * dirload_service_set::load_reload_service(const char *name, serv } file_input_stack input_stack; - input_stack.push(service_filename, std::move(service_file)); + input_stack.push(std::move(service_filename), std::move(service_file)); try { process_service_file(name, input_stack, [&](string &line, file_pos_ref fpr, string &setting, setting_op_t op, string_iterator &i, string_iterator &end) -> void { - auto process_dep_dir_n = [&](std::list &deplist, const std::string &waitsford, - dependency_type dep_type) -> void { - process_dep_dir(*this, name, service_filename, deplist, waitsford, dep_type, reload_svc); + auto process_dep_dir_n = [&](std::list &deplist, + const std::string &waitsford, dependency_type dep_type) -> void { + const string &service_filename = input_stack.current_file_name(); + process_dep_dir(*this, name, service_filename, deplist, waitsford, + dep_type, reload_svc); }; auto load_service_n = [&](const string &dep_name) -> service_record * { From 39c3047cf01c7219609a6d93ba875b41a294bb66 Mon Sep 17 00:00:00 2001 From: Davin McCall Date: Sat, 14 Dec 2024 18:28:37 +1000 Subject: [PATCH 22/51] Doc: add note on resolution of relative paths in service descriptions --- doc/manpages/README | 7 +++++-- doc/manpages/dinit-service.5.m4 | 8 ++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/doc/manpages/README b/doc/manpages/README index 3638a219..214f3e0a 100644 --- a/doc/manpages/README +++ b/doc/manpages/README @@ -15,7 +15,7 @@ Inline formatting ("escapes"): \fR - set regular (i.e. unset bold & italic) -Special characters: +Special characters (see groff_char(7)): '\ ' - (backslash followed by space) - non-breaking space \& - zero-width space \- - literal hyphen (also suppresses line breaking through dash) @@ -23,6 +23,9 @@ Special characters: \(en - en-dash \(bu - bullet point +` and ' (backtick / apostrophe) - use for opening/closing single quotes + + Line commands ("requests"): .\" - begins a comment line (may be used for spacing in source document without the effect that having a @@ -60,7 +63,7 @@ This should be followed by a NAME section: Gotchas: -* Sentences should always end at the end of a line (this affects spacing in output). +* Sentences should always end (with a period) at the end of a line (this affects spacing in output). * hyphens which are actually part of a word (part of a setting name for example) should be preceded by a backslash ("\-") which will prevent the literal from being split over multiple lines. * Blank lines should be avoided. Use .LP/.HP/.TP/.IP for example to start a paragraph. Use .IP for diff --git a/doc/manpages/dinit-service.5.m4 b/doc/manpages/dinit-service.5.m4 index 1586c5ab..bbd633d0 100644 --- a/doc/manpages/dinit-service.5.m4 +++ b/doc/manpages/dinit-service.5.m4 @@ -145,6 +145,14 @@ Setting a property generally overrides any previous setting (from prior lines). However some properties are set additively; these include dependency relationships and \fBoptions\fR properties. .LP +Some properties that specify file paths are currently resolved (if the specified path is relative) +starting from the directory containing the top-level service description file, whereas others are +resolved from the directory containing the service description fragment in which the setting value +is defined (a "fragment" may be the service description file itself, or it may be a file included +via \fB@include\fR or similar; see \fBMETA-COMMANDS\fR). In particular, the `\-\-\-.d' settings +(such as \fBwaits-for.d\fR) are resolved from the containing fragment. For all other settings, it +is recommended to provide absolute paths to be robust against future changes in Dinit. +.LP The following properties can be specified: .TP \fBtype\fR = {process | bgprocess | scripted | internal | triggered} From 8d50235737967807020877eecb6047eab2682a99 Mon Sep 17 00:00:00 2001 From: Davin McCall Date: Sun, 15 Dec 2024 14:04:05 +1000 Subject: [PATCH 23/51] Version 0.19.2 --- NEWS | 18 ++++++++++++++++++ README.md | 2 +- build/version.conf | 4 ++-- 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/NEWS b/NEWS index 228aed11..9aa8d6c4 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,21 @@ +== Version 0.19.2 (beta release #6) + +This beta release contains minor bugfixes compared to the prior release. It does not contain +various feature changes that be released in version 0.20.0 in due course. + +Thanks to current sponsors: Paweł Zmarzły (pzmarzly), Wesley Moore (wezm), +M. Herdiansyah (konimex), Coleman McFarland (dontlaugh), q66, saolof, and private sponsors. + +Fixes: + * Fix bad formatting of line number in error message when a service fails to load (Jami Kettunen). + * Fix endianness issue leading to errors from dinitctl for some commands on big-endian systems + (q66). + * Fix dinitcheck not being able to find dependency directories ("waits-for.d" etc) if specified + via relative path. + * Dependency directories (as specified by "waits-for.d" and similar) are, if relative, now + resolved against the file containing the setting, rather than the base service description + file. It is recommended to use absolute paths for all other applicable settings. + == Version 0.19.1 (beta release #5) This is a fifth beta release, containing important bugfixes compared to the prior release. diff --git a/README.md b/README.md index fe18fd78..77e0b05d 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # Dinit -v0.19.1 +v0.19.2 --- ![Dinit logo](doc/dinit-logo.png) diff --git a/build/version.conf b/build/version.conf index 934f1d3a..04aec1f0 100644 --- a/build/version.conf +++ b/build/version.conf @@ -1,4 +1,4 @@ # Included from Makefiles. -VERSION=0.19.1 -MONTH=October +VERSION=0.19.2 +MONTH=December YEAR=2024 From 53b0ae4d3cc6a2f620a39fe385bab73bfcf169f8 Mon Sep 17 00:00:00 2001 From: q66 Date: Wed, 2 Oct 2024 01:53:18 +0200 Subject: [PATCH 24/51] Add support for services with an argument Fixes https://github.com/davmac314/dinit/issues/385 --- doc/manpages/dinit-service.5.m4 | 36 +++++++++++++-- doc/manpages/dinitctl.8.m4 | 1 + src/dinitcheck.cc | 4 +- src/igr-tests/igr-runner.cc | 24 +++++++++- src/igr-tests/svc-arg/checkarg.sh | 3 ++ src/igr-tests/svc-arg/sd/checkarg | 4 ++ src/igr-tests/svc-arg/sd/checkarg2 | 3 ++ src/igr-tests/svc-arg/sd/checkarg3 | 3 ++ src/igr-tests/svc-arg/sd/checkarg4 | 3 ++ src/igr-tests/svc-arg/sd/checkarg5 | 2 + src/includes/load-service.h | 70 ++++++++++++++++++++++-------- src/load-service.cc | 49 ++++++++++++--------- src/tests/loadtests.cc | 38 ++++++++-------- 13 files changed, 176 insertions(+), 64 deletions(-) create mode 100755 src/igr-tests/svc-arg/checkarg.sh create mode 100644 src/igr-tests/svc-arg/sd/checkarg create mode 100644 src/igr-tests/svc-arg/sd/checkarg2 create mode 100644 src/igr-tests/svc-arg/sd/checkarg3 create mode 100644 src/igr-tests/svc-arg/sd/checkarg4 create mode 100644 src/igr-tests/svc-arg/sd/checkarg5 diff --git a/doc/manpages/dinit-service.5.m4 b/doc/manpages/dinit-service.5.m4 index bbd633d0..f614bc23 100644 --- a/doc/manpages/dinit-service.5.m4 +++ b/doc/manpages/dinit-service.5.m4 @@ -12,15 +12,23 @@ Dinit service description files .SH DESCRIPTION .\" The service description files for \fBDinit\fR each describe a service. The name -of the file corresponds to the name of the service it describes. +of the file corresponds to the name of the service it describes, minus its argument. .LP Service description files specify the various attributes of a service. A -service description file is named after the service it represents, and is -a plain-text file with simple key-value format. +service description file is named after the service it represents (without +its argument), and is a plain-text file with simple key-value format. The description files are located in a service description directory; See \fBdinit\fR(8) for more details of the default service description directories, and how and when service descriptions are loaded. .LP +The full name of the service includes its argument, such as \fIservice@argument\fR. +The argument is optional, so you can also invoke just \fIservice\fR. +Each instance of a service, i.e. with different arguments, is separate, including loading. +This means every time you invoke the service with a different argument, it is loaded +separately. +Empty argument is not the same as missing argument, as this affects variable +substitution (see \fBVARIABLE SUBSTITUTION\fR). +.LP All services have a \fItype\fR and a set of \fIdependencies\fR. These are discussed in the following subsections. The type, dependencies, and other attributes are specified via property settings, the format of which are documented in the @@ -277,6 +285,8 @@ This service depends on the named service. Starting this service will start the named service; the command to start this service will not be executed until the named service has started. If the named service stops then this service will also be stopped. +The \fIservice-name\fR is subject to minimal variable substitution +(see \fBVARIABLE SUBSTITUTION\fR). .TP \fBdepends\-ms\fR: \fIservice-name\fR This service has a "milestone" dependency on the named service. Starting this @@ -285,6 +295,7 @@ named service has started, and will fail to start if the named service does not start. Once the named (dependent) service reaches the started state, however, the dependency may stop without affecting the dependent service. +The name is likewise subject to minimal variable substitution. .TP \fBwaits\-for\fR: \fIservice-name\fR When this service is started, wait for the named service to finish starting @@ -292,6 +303,7 @@ When this service is started, wait for the named service to finish starting Starting this service will automatically start the named service. If the named service fails to start, this service will start as usual (subject to other dependencies being met). +The name is likewise subject to minimal variable substitution. .TP \fBdepends\-on.d\fR: \fIdirectory-path\fR For each file name in \fIdirectory-path\fR which does not begin with a dot, @@ -304,6 +316,7 @@ is not considered fatal. .IP The directory path, if not absolute, is relative to the directory containing the service description file. +No variable substitution is done for path dependencies. .TP \fBdepends\-ms.d\fR: \fIdirectory-path\fR As for \fBdepends-on.d\fR, but with dependency type \fBdepends\-ms\fR. @@ -319,6 +332,8 @@ starting this service will not cause it to start (nor wait for it in that case). It does not by itself cause the named service to be loaded (if loaded later, the "after" relationship will be enforced from that point). .TP +The name is subject to minimal variable substitution. +.TP \fBbefore\fR: \fIservice-name\fR When starting the named service, if this service is also starting, wait for this service to finish starting before bringing the named service up. This is largely equivalent to specifying @@ -326,6 +341,8 @@ an \fBafter\fR relationship to this service from the named service. However, it does not by itself cause the named service to be loaded (if loaded later, the "before" relationship will be enforced from that point). .TP +The name is subject to minimal variable substitution. +.TP \fBchain\-to\fR = \fIservice-name\fR When this service terminates (i.e. starts successfully, and then stops of its own accord), the named service should be started. @@ -346,6 +363,8 @@ stopped due to a dependency stopping (for any reason), if it will restart abnormally or with an exit status indicating an error. However, if the \fBalways-chain\fR option is set the chain is started regardless of the reason and the status of this service termination. +.IP +The name is subject to minimal variable substitution. .TP \fBsocket\-listen\fR = \fIsocket-path\fR Pre-open a socket for the service and pass it to the service using the @@ -719,6 +738,11 @@ set and non\-empty), and `\fB${NAME+word}\fR' (substitute `\fBword\fR' if variab Unlike in shell expansion, the substituted \fBword\fR does not itself undergo expansion and cannot contain closing brace characters or whitespace, even if quoted. .LP +To substitute the service argument, the `\fB$1\fR' syntax may be used. +The complete syntax of the substitution is supported here. +Services without an argument are treated as if the variable was unset, which +affects some of the curly brace syntax variants. +.LP Note that by default, command-line variable substitution occurs after splitting the line into separate arguments and so a single environment variable cannot be used to add multiple arguments to a command line. @@ -749,6 +773,12 @@ used for substitution, if they have been changed in the meantime. Using environment variable values in service commands and parameters can be used as means to provide easily-adjustable service configuration, but is not ideal for this purpose and alternatives should be considered. +.LP +In dependency fields, including \fIbefore\fR and similar, minimal version of variable +substitution may happen. +Only the service argument may be substituted, as the actual environment is not available +at this point. +The full syntax is still supported. .\" .SS META-COMMANDS .\" diff --git a/doc/manpages/dinitctl.8.m4 b/doc/manpages/dinitctl.8.m4 index e148de87..3674a741 100644 --- a/doc/manpages/dinitctl.8.m4 +++ b/doc/manpages/dinitctl.8.m4 @@ -163,6 +163,7 @@ Clear the log buffer for the service after displaying it. .TP \fIservice-name\fR Specifies the name of the service to which the command applies. +It may have an argument that is passed to the service. .\" .SH COMMAND DESCRIPTIONS .\" diff --git a/src/dinitcheck.cc b/src/dinitcheck.cc index 577633d7..ef45f9d8 100644 --- a/src/dinitcheck.cc +++ b/src/dinitcheck.cc @@ -672,7 +672,7 @@ service_record *load_service(service_set_t &services, const std::string &name, }; try { - process_service_line(settings, name.c_str(), line, input_pos, setting, op, i, end, + process_service_line(settings, name.c_str(), nullptr, line, input_pos, setting, op, i, end, load_service_n, process_dep_dir_n); } catch (service_description_exc &exc) { @@ -746,7 +746,7 @@ service_record *load_service(service_set_t &services, const std::string &name, return resolve_env_var(name, envmap); }; - settings.finalise(report_err, renvmap, report_err, resolve_var); + settings.finalise(report_err, renvmap, nullptr, report_err, resolve_var); if (!settings.working_dir.empty()) { service_wdir = settings.working_dir; diff --git a/src/igr-tests/igr-runner.cc b/src/igr-tests/igr-runner.cc index c87731b0..662a7757 100644 --- a/src/igr-tests/igr-runner.cc +++ b/src/igr-tests/igr-runner.cc @@ -40,6 +40,7 @@ void catlog_test(); void offline_enable_test(); void xdg_config_test(); void cycles_test(); +void svc_arg_test(); int main(int argc, char **argv) { @@ -55,7 +56,8 @@ int main(int argc, char **argv) { "pseudo-cycle", pseudo_cycle_test }, { "before-after", before_after_test}, { "before-after2", before_after2_test }, { "log-via-pipe", log_via_pipe_test }, { "catlog", catlog_test }, { "offline-enable", offline_enable_test }, - { "xdg-config", xdg_config_test }, { "cycles", cycles_test } }; + { "xdg-config", xdg_config_test }, { "cycles", cycles_test }, + { "svc-arg", svc_arg_test } }; constexpr int num_tests = sizeof(tests) / sizeof(tests[0]); dinit_bindir = "../.."; @@ -974,3 +976,23 @@ void cycles_test() igr_assert_eq(read_file_contents(igr_input_basedir + "/cycles/expected-after_self"), dinitctl_p.get_stderr()); } + +void svc_arg_test() +{ + igr_test_setup setup("svc-arg"); + std::string output_file = setup.prep_output_file("svc-arg-record"); + std::string socket_path = setup.prep_socket_path(); + + igr_env_var_setup env_output("OUTPUT", output_file.c_str()); + + dinit_proc dinit_p; + dinit_p.start("svc-arg", {"-u", "-d", "sd", "-p", socket_path, "-q", "checkarg@foo"}); + dinit_p.wait_for_term({1,0} /* max 1 second */); + + check_file_contents(output_file, std::string() + + "test-$1\n" + + "test-hello\n" + + "hello\n" + + "foo\n" + + "foo\n"); +} diff --git a/src/igr-tests/svc-arg/checkarg.sh b/src/igr-tests/svc-arg/checkarg.sh new file mode 100755 index 00000000..f6bff94b --- /dev/null +++ b/src/igr-tests/svc-arg/checkarg.sh @@ -0,0 +1,3 @@ +#!/bin/sh + +echo "$1" >> "$OUTPUT" diff --git a/src/igr-tests/svc-arg/sd/checkarg b/src/igr-tests/svc-arg/sd/checkarg new file mode 100644 index 00000000..d781ef9c --- /dev/null +++ b/src/igr-tests/svc-arg/sd/checkarg @@ -0,0 +1,4 @@ +type = process +waits-for = checkarg2@$1 +command = ../checkarg.sh $1 +restart = false diff --git a/src/igr-tests/svc-arg/sd/checkarg2 b/src/igr-tests/svc-arg/sd/checkarg2 new file mode 100644 index 00000000..e8b58df5 --- /dev/null +++ b/src/igr-tests/svc-arg/sd/checkarg2 @@ -0,0 +1,3 @@ +type = scripted +waits-for = checkarg3@hello +command = ../checkarg.sh $1 diff --git a/src/igr-tests/svc-arg/sd/checkarg3 b/src/igr-tests/svc-arg/sd/checkarg3 new file mode 100644 index 00000000..0521f6fb --- /dev/null +++ b/src/igr-tests/svc-arg/sd/checkarg3 @@ -0,0 +1,3 @@ +type = scripted +waits-for = checkarg4@test-$1 +command = ../checkarg.sh $1 diff --git a/src/igr-tests/svc-arg/sd/checkarg4 b/src/igr-tests/svc-arg/sd/checkarg4 new file mode 100644 index 00000000..7ef74878 --- /dev/null +++ b/src/igr-tests/svc-arg/sd/checkarg4 @@ -0,0 +1,3 @@ +type = scripted +waits-for = checkarg5@test-$$1 +command = ../checkarg.sh $1 diff --git a/src/igr-tests/svc-arg/sd/checkarg5 b/src/igr-tests/svc-arg/sd/checkarg5 new file mode 100644 index 00000000..26ee0c21 --- /dev/null +++ b/src/igr-tests/svc-arg/sd/checkarg5 @@ -0,0 +1,2 @@ +type = scripted +command = ../checkarg.sh $1 diff --git a/src/includes/load-service.h b/src/includes/load-service.h index ff31c93a..333e336c 100644 --- a/src/includes/load-service.h +++ b/src/includes/load-service.h @@ -373,7 +373,7 @@ inline int signal_name_to_number(const std::string &signame) noexcept // // If env is set, dashes/dots are not allowed within names. They are not typically allowed by shells // and they interfere with substitution patterns. -inline string read_config_name(string_iterator & i, string_iterator end, bool env = false) noexcept +inline string read_config_name(string_iterator & i, string_iterator end, bool env = false, bool *num = nullptr) noexcept { using std::locale; using std::ctype; @@ -388,6 +388,18 @@ inline string read_config_name(string_iterator & i, string_iterator end, bool en string rval; + // For environment lookups, integers are valid names (particularly for argument) + if (env && facet.is(ctype::digit, *i)) { + while (facet.is(ctype::digit, *i)) { + rval += *i; + ++i; + } + if (num) { + *num = true; + } + return rval; + } + // Don't allow empty name, numeric digit, or dash/dot at start of setting name if (i == end || (*i == '-' || *i == '.' || facet.is(ctype::digit, *i))) { return {}; @@ -1031,7 +1043,7 @@ inline const char *resolve_env_var(const string &name, const environment::env_ma template static void value_var_subst(const char *setting_name, std::string &line, std::list> &offsets, T &var_resolve, - environment::env_map const &envmap) + environment::env_map const *envmap, const char *argval) { auto dindx = line.find('$'); if (dindx == string::npos) { @@ -1074,11 +1086,17 @@ static void value_var_subst(const char *setting_name, std::string &line, bool brace = line[spos] == '{'; if (brace) ++spos; auto j = std::next(line.begin(), spos); + // may be a service argument + bool is_arg = false; // read environment variable name - string name = read_config_name(j, token_end, true); + string name = read_config_name(j, token_end, true, &is_arg); if (name.empty()) { throw service_description_exc(setting_name, "invalid/missing variable name after '$'"); } + else if (is_arg && (name != "1" || !argval)) { + // only one arg is supported and it must be present + throw service_description_exc(setting_name, "missing value in argument substitution"); + } char altmode = '\0'; bool colon = false; auto altbeg = j, altend = j; @@ -1106,7 +1124,7 @@ static void value_var_subst(const char *setting_name, std::string &line, } size_t line_len_before = r_line.size(); string_view resolved_vw; - auto *resolved = var_resolve(name, envmap); + auto *resolved = is_arg ? argval : (envmap ? var_resolve(name, *envmap) : nullptr); if (resolved) { resolved_vw = resolved; } @@ -1215,6 +1233,18 @@ static void value_var_subst(const char *setting_name, std::string &line, line = std::move(r_line); } +// Reads a dependency name while performing minimal argument expansion in it. +inline string read_dependency_value(const char *setting_name, file_pos_ref input_pos, string_iterator &i, + string_iterator end, std::list> &offsets, const char *argval) +{ + string rval; + read_setting_value(rval, setting_op_t::ASSIGN, input_pos, i, end, nullptr); + offsets.clear(); + offsets.emplace_back(0, rval.size()); + value_var_subst(setting_name, rval, offsets, resolve_env_var, nullptr, argval); + return rval; +} + // A wrapper type for service parameters. It is parameterised by dependency type. template class service_settings_wrapper @@ -1239,6 +1269,7 @@ class service_settings_wrapper list depends; list before_svcs; list after_svcs; + list> str_offsets; // stores offsets for any substutions where we don't care about them log_type_id log_type = log_type_id::NONE; string logfile; int logfile_perms = 0600; @@ -1300,7 +1331,7 @@ class service_settings_wrapper // var_subst - functor to resolve environment variable values template ::value> - void finalise(T &report_error, environment::env_map const &envmap, U &report_lint = dummy_lint, V &var_subst = resolve_env_var) + void finalise(T &report_error, environment::env_map const &envmap, const char *argval, U &report_lint = dummy_lint, V &var_subst = resolve_env_var) { if (service_type == service_type_t::PROCESS || service_type == service_type_t::BGPROCESS || service_type == service_type_t::SCRIPTED) { @@ -1377,14 +1408,11 @@ class service_settings_wrapper // Resolve paths via variable substitution { - std::list> offsets; - /* reserve item */ - offsets.emplace_back(0, 0); auto do_resolve = [&](const char *setting_name, string &setting_value) { try { - offsets.front().first = 0; - offsets.front().second = setting_value.size(); - value_var_subst(setting_name, setting_value, offsets, var_subst, envmap); + str_offsets.clear(); + str_offsets.emplace_back(0, setting_value.size()); + value_var_subst(setting_name, setting_value, str_offsets, var_subst, &envmap, argval); } catch (service_description_exc &exc) { if (propagate_sde) throw; @@ -1460,7 +1488,7 @@ class service_settings_wrapper template -void process_service_line(settings_wrapper &settings, const char *name, string &line, +void process_service_line(settings_wrapper &settings, const char *name, const char *arg, string &line, file_pos_ref input_pos, string &setting, setting_op_t setting_op, string::iterator &i, string::iterator &end, load_service_t load_service, process_dep_dir_t process_dep_dir) @@ -1524,20 +1552,24 @@ void process_service_line(settings_wrapper &settings, const char *name, string & break; case setting_id_t::DEPENDS_ON: { - string dependency_name = read_setting_value(input_pos, i, end); - settings.depends.emplace_back(load_service(dependency_name.c_str()), dependency_type::REGULAR); + string dependency_name = read_dependency_value(setting.c_str(), input_pos, + i, end, settings.str_offsets, arg); + settings.depends.emplace_back(load_service(dependency_name.c_str()), + dependency_type::REGULAR); break; } case setting_id_t::DEPENDS_MS: { - string dependency_name = read_setting_value(input_pos, i, end); + string dependency_name = read_dependency_value(setting.c_str(), input_pos, + i, end, settings.str_offsets, arg); settings.depends.emplace_back(load_service(dependency_name.c_str()), dependency_type::MILESTONE); break; } case setting_id_t::WAITS_FOR: { - string dependency_name = read_setting_value(input_pos, i, end); + string dependency_name = read_dependency_value(setting.c_str(), input_pos, + i, end, settings.str_offsets, arg); settings.depends.emplace_back(load_service(dependency_name.c_str()), dependency_type::WAITS_FOR); break; @@ -1562,13 +1594,15 @@ void process_service_line(settings_wrapper &settings, const char *name, string & } case setting_id_t::AFTER: { - string after_name = read_setting_value(input_pos, i, end); + string after_name = read_dependency_value(setting.c_str(), input_pos, + i, end, settings.str_offsets, arg); settings.after_svcs.emplace_back(std::move(after_name)); break; } case setting_id_t::BEFORE: { - string before_name = read_setting_value(input_pos, i, end); + string before_name = read_dependency_value(setting.c_str(), input_pos, + i, end, settings.str_offsets, arg); settings.before_svcs.emplace_back(std::move(before_name)); break; } diff --git a/src/load-service.cc b/src/load-service.cc index 7737a1c0..acfb4ca8 100644 --- a/src/load-service.cc +++ b/src/load-service.cc @@ -31,11 +31,11 @@ using string_iterator = std::string::iterator; // throws: std::bad_alloc, std::length_error, service_description_exc static void do_env_subst(const char *setting_name, ha_string &line, std::list> &offsets, - environment::env_map const &envmap) + environment::env_map const &envmap, const char *arg) { using namespace dinit_load; std::string line_s = std::string(line.c_str(), line.length()); - value_var_subst(setting_name, line_s, offsets, resolve_env_var, envmap); + value_var_subst(setting_name, line_s, offsets, resolve_env_var, &envmap, arg); line = line_s; } @@ -311,7 +311,7 @@ static bool check_settings_for_reload(service_record *service, return create_new_record; } -service_record * dirload_service_set::load_reload_service(const char *name, service_record *reload_svc, +service_record * dirload_service_set::load_reload_service(const char *fullname, service_record *reload_svc, const service_record *avoid_circular) { // Load a new service, or reload an already-loaded service. @@ -366,9 +366,16 @@ service_record * dirload_service_set::load_reload_service(const char *name, serv using namespace dinit_load; + const auto *argp = strchr(fullname, '@'); + if (!argp) argp = fullname + strlen(fullname); + + auto name = string(fullname, argp); + + auto *argval = *argp ? argp + 1 : nullptr; + if (reload_svc == nullptr) { // First try and find an existing record... - service_record *existing = find_service(string(name), true); + service_record *existing = find_service(string(fullname), true); if (existing != nullptr) { if (existing == avoid_circular || existing->check_is_loading()) { throw service_cyclic_dependency(name); @@ -412,7 +419,7 @@ service_record * dirload_service_set::load_reload_service(const char *name, serv if (!service_file) { if (fail_load_errno == 0) { - throw service_not_found(string(name)); + throw service_not_found(name); } else { throw service_load_error(name, std::move(fail_load_path), fail_load_errno); @@ -476,7 +483,7 @@ service_record * dirload_service_set::load_reload_service(const char *name, serv // Add a placeholder record now to prevent infinite recursion in case of cyclic dependency. // We replace this with the real service later (or remove it if we find a configuration error). try { - dummy = new service_record(this, string(name), service_record::LOADING_TAG); + dummy = new service_record(this, string(fullname), service_record::LOADING_TAG); add_service(dummy); } catch (...) { @@ -497,7 +504,7 @@ service_record * dirload_service_set::load_reload_service(const char *name, serv auto process_dep_dir_n = [&](std::list &deplist, const std::string &waitsford, dependency_type dep_type) -> void { const string &service_filename = input_stack.current_file_name(); - process_dep_dir(*this, name, service_filename, deplist, waitsford, + process_dep_dir(*this, name.c_str(), service_filename, deplist, waitsford, dep_type, reload_svc); }; @@ -516,8 +523,8 @@ service_record * dirload_service_set::load_reload_service(const char *name, serv } }; - process_service_line(settings, name, line, fpr, setting, op, i, end, load_service_n, - process_dep_dir_n); + process_service_line(settings, name.c_str(), argval, line, fpr, setting, + op, i, end, load_service_n, process_dep_dir_n); }); auto report_err = [&](const char *msg){ @@ -560,7 +567,7 @@ service_record * dirload_service_set::load_reload_service(const char *name, serv srv_envmap = srv_env.build(main_env); - settings.finalise(report_err, srv_envmap); + settings.finalise(report_err, srv_envmap, argval); auto service_type = settings.service_type; if (reload_svc != nullptr) { @@ -704,15 +711,15 @@ service_record * dirload_service_set::load_reload_service(const char *name, serv } if (service_type == service_type_t::PROCESS) { - do_env_subst("command", settings.command, settings.command_offsets, srv_envmap); - do_env_subst("stop-command", settings.stop_command, settings.stop_command_offsets, srv_envmap); + do_env_subst("command", settings.command, settings.command_offsets, srv_envmap, argval); + do_env_subst("stop-command", settings.stop_command, settings.stop_command_offsets, srv_envmap, argval); std::vector stop_arg_parts = separate_args(settings.stop_command, settings.stop_command_offsets); process_service *rvalps; if (create_new_record) { if (reload_svc != nullptr) { check_cycle(settings.depends, reload_svc); } - rvalps = new process_service(this, string(name), std::move(settings.command), + rvalps = new process_service(this, string(fullname), std::move(settings.command), settings.command_offsets, settings.depends); settings.depends.clear(); } @@ -747,15 +754,15 @@ service_record * dirload_service_set::load_reload_service(const char *name, serv #endif } else if (service_type == service_type_t::BGPROCESS) { - do_env_subst("command", settings.command, settings.command_offsets, srv_envmap); - do_env_subst("stop-command", settings.stop_command, settings.stop_command_offsets, srv_envmap); + do_env_subst("command", settings.command, settings.command_offsets, srv_envmap, argval); + do_env_subst("stop-command", settings.stop_command, settings.stop_command_offsets, srv_envmap, argval); std::vector stop_arg_parts = separate_args(settings.stop_command, settings.stop_command_offsets); bgproc_service *rvalps; if (create_new_record) { if (reload_svc != nullptr) { check_cycle(settings.depends, reload_svc); } - rvalps = new bgproc_service(this, string(name), std::move(settings.command), + rvalps = new bgproc_service(this, string(fullname), std::move(settings.command), settings.command_offsets, settings.depends); settings.depends.clear(); } @@ -786,15 +793,15 @@ service_record * dirload_service_set::load_reload_service(const char *name, serv settings.onstart_flags.runs_on_console = false; } else if (service_type == service_type_t::SCRIPTED) { - do_env_subst("command", settings.command, settings.command_offsets, srv_envmap); - do_env_subst("stop-command", settings.stop_command, settings.stop_command_offsets, srv_envmap); + do_env_subst("command", settings.command, settings.command_offsets, srv_envmap, argval); + do_env_subst("stop-command", settings.stop_command, settings.stop_command_offsets, srv_envmap, argval); std::vector stop_arg_parts = separate_args(settings.stop_command, settings.stop_command_offsets); scripted_service *rvalps; if (create_new_record) { if (reload_svc != nullptr) { check_cycle(settings.depends, reload_svc); } - rvalps = new scripted_service(this, string(name), std::move(settings.command), + rvalps = new scripted_service(this, string(fullname), std::move(settings.command), settings.command_offsets, settings.depends); settings.depends.clear(); } @@ -826,11 +833,11 @@ service_record * dirload_service_set::load_reload_service(const char *name, serv check_cycle(settings.depends, reload_svc); } if (service_type == service_type_t::INTERNAL) { - rval = new service_record(this, string(name), service_type, settings.depends); + rval = new service_record(this, string(fullname), service_type, settings.depends); } else { /* TRIGGERED */ - rval = new triggered_service(this, string(name), service_type, settings.depends); + rval = new triggered_service(this, string(fullname), service_type, settings.depends); } settings.depends.clear(); } diff --git a/src/tests/loadtests.cc b/src/tests/loadtests.cc index 44e0f073..aea0c698 100644 --- a/src/tests/loadtests.cc +++ b/src/tests/loadtests.cc @@ -77,7 +77,7 @@ void test_env_subst2() dinit_load::read_setting_value(fpr, li, le, &offsets); - dinit_load::value_var_subst("command", line, offsets, resolve_env_var, tenvmap); + dinit_load::value_var_subst("command", line, offsets, resolve_env_var, &tenvmap, nullptr); assert(line == "test xa-a~ y${TWOVAR}hellohello$ONE_VAR"); @@ -104,7 +104,7 @@ void test_env_subst3() std::string::iterator li = line.begin(); std::string::iterator le = line.end(); dinit_load::read_setting_value(fpr, li, le, &offsets); - dinit_load::value_var_subst("command", line, offsets, resolve_env_var, tenvmap); + dinit_load::value_var_subst("command", line, offsets, resolve_env_var, &tenvmap, nullptr); auto check_arg = [&](unsigned idx, const char *val) { @@ -118,7 +118,7 @@ void test_env_subst3() line = "test $EMPTY foo"; li = line.begin(); le = line.end(); offsets.clear(); dinit_load::read_setting_value(fpr, li, le, &offsets); - dinit_load::value_var_subst("command", line, offsets, resolve_env_var, tenvmap); + dinit_load::value_var_subst("command", line, offsets, resolve_env_var, &tenvmap, nullptr); assert(line == "test foo"); check_arg(1, ""); @@ -128,7 +128,7 @@ void test_env_subst3() line = "test $/EMPTY$/EMPTY$/EMPTY foo"; li = line.begin(); le = line.end(); offsets.clear(); dinit_load::read_setting_value(fpr, li, le, &offsets); - dinit_load::value_var_subst("command", line, offsets, resolve_env_var, tenvmap); + dinit_load::value_var_subst("command", line, offsets, resolve_env_var, &tenvmap, nullptr); assert(line == "test foo"); check_arg(1, "foo"); @@ -137,7 +137,7 @@ void test_env_subst3() line = "test $/EMPTY$EMPTY$/EMPTY foo"; li = line.begin(); le = line.end(); offsets.clear(); dinit_load::read_setting_value(fpr, li, le, &offsets); - dinit_load::value_var_subst("command", line, offsets, resolve_env_var, tenvmap); + dinit_load::value_var_subst("command", line, offsets, resolve_env_var, &tenvmap, nullptr); assert(line == "test foo"); check_arg(1, ""); @@ -147,7 +147,7 @@ void test_env_subst3() line = "test abc$/{EMPTY}def"; li = line.begin(); le = line.end(); offsets.clear(); dinit_load::read_setting_value(fpr, li, le, &offsets); - dinit_load::value_var_subst("command", line, offsets, resolve_env_var, tenvmap); + dinit_load::value_var_subst("command", line, offsets, resolve_env_var, &tenvmap, nullptr); assert(line == "test abcdef"); check_arg(1, "abcdef"); @@ -156,7 +156,7 @@ void test_env_subst3() line = "test abc$/{WS}def"; li = line.begin(); le = line.end(); offsets.clear(); dinit_load::read_setting_value(fpr, li, le, &offsets); - dinit_load::value_var_subst("command", line, offsets, resolve_env_var, tenvmap); + dinit_load::value_var_subst("command", line, offsets, resolve_env_var, &tenvmap, nullptr); assert(line == "test abc def"); check_arg(1, "abc"); @@ -166,7 +166,7 @@ void test_env_subst3() line = "test abc$/{PADDED}def"; li = line.begin(); le = line.end(); offsets.clear(); dinit_load::read_setting_value(fpr, li, le, &offsets); - dinit_load::value_var_subst("command", line, offsets, resolve_env_var, tenvmap); + dinit_load::value_var_subst("command", line, offsets, resolve_env_var, &tenvmap, nullptr); assert(line == "test abc p def"); check_arg(1, "abc"); @@ -237,8 +237,8 @@ void test_settings() }; try { - process_service_line(settings, "test-service", line, input_pos, setting, op, i, end, - load_service_n, process_dep_dir_n); + process_service_line(settings, "test-service", nullptr, line, input_pos, setting, + op, i, end, load_service_n, process_dep_dir_n); } catch (service_description_exc &exc) { //report_service_description_exc(exc); @@ -287,7 +287,7 @@ void test_path_env_subst() ss << "type = process\n" "command = /something/test\n" - "logfile = /some/$username/dir\n"; + "logfile = /some/$1/$username/${1}/dir\n"; file_input_stack input_stack; input_stack.add_source(ss.str(), "dummy"); @@ -307,8 +307,8 @@ void test_path_env_subst() }; try { - process_service_line(settings, "test-service", line, input_pos, setting, op, i, - end, load_service_n, process_dep_dir_n); + process_service_line(settings, "test-service", nullptr, line, input_pos, setting, + op, i, end, load_service_n, process_dep_dir_n); } catch (service_description_exc &exc) { //report_service_description_exc(exc); @@ -327,11 +327,11 @@ void test_path_env_subst() return nullptr; }; - settings.finalise(report_error, tenvmap, report_error /* lint */, resolve_var); + settings.finalise(report_error, tenvmap, "foo", report_error /* lint */, resolve_var); assert(settings.service_type == service_type_t::PROCESS); assert(settings.command == "/something/test"); - assert(settings.logfile == "/some/testsuccess/dir"); + assert(settings.logfile == "/some/foo/testsuccess/foo/dir"); } void test_newline() @@ -381,8 +381,8 @@ void test_newline_err() return dep_name; }; - process_service_line(settings, "test-service", line, input_pos, setting, op, i, end, - load_service_n, process_dep_dir_n); + process_service_line(settings, "test-service", nullptr, line, input_pos, setting, + op, i, end, load_service_n, process_dep_dir_n); }); }; @@ -448,8 +448,8 @@ void test_newline2() }; try { - process_service_line(settings, "test-service", line, input_pos, setting, op, i, end, - load_service_n, process_dep_dir_n); + process_service_line(settings, "test-service", nullptr, line, input_pos, + setting, op, i, end, load_service_n, process_dep_dir_n); } catch (service_description_exc &exc) { //report_service_description_exc(exc); From 61a16c3034444bc522be7e33db314ee8f3cfb1c9 Mon Sep 17 00:00:00 2001 From: q66 Date: Thu, 3 Oct 2024 22:17:43 +0200 Subject: [PATCH 25/51] Implement environment unset and listening on environment changes --- doc/manpages/dinit-monitor.8.m4 | 35 ++++-- doc/manpages/dinitctl.8.m4 | 7 ++ src/control.cc | 50 +++++++- src/dinit-monitor.cc | 206 +++++++++++++++++++++++++++++--- src/dinitctl.cc | 26 ++-- src/igr-tests/environ/setenv.sh | 3 +- src/igr-tests/igr-runner.cc | 1 + src/includes/control-cmds.h | 8 +- src/includes/control.h | 9 +- src/includes/dinit-env.h | 70 ++++++++++- src/tests/cptests/cptests.cc | 109 +++++++++++++++++ 11 files changed, 481 insertions(+), 43 deletions(-) diff --git a/doc/manpages/dinit-monitor.8.m4 b/doc/manpages/dinit-monitor.8.m4 index 2cdd48b3..6de49e31 100644 --- a/doc/manpages/dinit-monitor.8.m4 +++ b/doc/manpages/dinit-monitor.8.m4 @@ -1,7 +1,7 @@ changequote(`@@@',`$$$')dnl @@@.TH DINIT\-MONITOR "8" "$$$MONTH YEAR@@@" "Dinit $$$VERSION@@@" "Dinit \- service management system" .SH NAME -dinit\-monitor \- monitor services supervised by Dinit +dinit\-monitor \- monitor services or environment supervised by Dinit .\" .SH SYNOPSIS .\" @@ -9,15 +9,17 @@ dinit\-monitor \- monitor services supervised by Dinit .nh .HP .B dinit-monitor -[\fIoptions\fR] {\fB\-c\fR \fIcommand\fR, \fB\-\-command\fR \fIcommand\fR} \fIservice-name\fR [\fIservice-name\fR...] +[\fIoptions\fR] {\fB\-c\fR \fIcommand\fR, \fB\-\-command\fR \fIcommand\fR} \fIservice-or-env\fR [\fIservice-or-env\fR...] .\" .PD .hy .\" .SH DESCRIPTION .\" -\fBdinit\-monitor\fR is a utility to monitor the state of one or more services managed by the \fBdinit\fR daemon. -Changes in service state are reported by the execution of the specified command. +\fBdinit\-monitor\fR is a utility to monitor the state of one or more services or environment managed by the \fBdinit\fR daemon. +Changes in service or environment state are reported by the execution of the specified command. +When monitoring environment, positional arguments are not required (all environment will be monitored). +By default, service events are monitored. .\" .SH GENERAL OPTIONS .TP @@ -27,6 +29,11 @@ Display brief help text and then exit. \fB\-\-version\fR Display version and then exit. .TP +\fB\-e\fR, \fB\-\-exit\fR +Exit after the first command is executed, instead of waiting for more events. +\fB\-E\fR, \fB\-\-env\fR +Instead of monitoring the services, monitor changes in the global environment. +If no environment variables are passed, all environment is monitored. \fB\-s\fR, \fB\-\-system\fR Control the system init process (this is the default when run as root). This option determines the default path to the control socket used to communicate with the \fBdinit\fR daemon @@ -38,8 +45,8 @@ This option determines the default path to the control socket used to communicat (it does not override the \fB\-p\fR option). .TP \fB\-i\fR, \fB\-\-initial\fR -Issue the specified command additionally for the initial status of the services (when \fBdinit\-monitor\fR is started). -Without this option, the command is only executed whenever service status changes. +Issue the specified command additionally for the initial status of the services or environment (when \fBdinit\-monitor\fR is started). +Without this option, the command is only executed whenever status changes. .TP \fB\-\-str\-started\fR \fIstarted-text\fR Specify the text used for the substitution of the status in the command (as specified @@ -53,6 +60,14 @@ by the \fB\-\-command\fR option) when a service stops. Specify the text used for the substitution of the status in the command (as specified by the \fB\-\-command\fR option) when a service fails to start. .TP +\fB\-\-str\-set\fR \fIset-text\fR +Specify the text used for the substitution of the status in the command (as specified +by the \fB\-\-command\fR option) when an environment variable is set. +.TP +\fB\-\-str\-unset\fR \fIunset-text\fR +Specify the text used for the substitution of the status in the command (as specified +by the \fB\-\-command\fR option) when an environment variable is unset. +.TP \fB\-\-socket\-path\fR \fIsocket-path\fR, \fB\-p\fR \fIsocket-path\fR Specify the path to the socket used for communicating with the service manager daemon. When not specified, the \fIDINIT_SOCKET_PATH\fR environment variable is read, otherwise @@ -61,9 +76,11 @@ Dinit's default values are used. .SH STATUS REPORT OPTIONS .TP \fB\-\-command\fR \fIcommand\fR, \fB\-c\fR \fIcommand\fR -Execute the specified \fIcommand\fR when the service status changes. In \fIcommand\fR, \fB%n\fR -will be substituted with the service name and \fB%s\fR will be substituted with a textual -description of the new status (\fBstarted\fR, \fBstopped\fR or \fBfailed\fR). A double percent sign +Execute the specified \fIcommand\fR when the service status changes. +In \fIcommand\fR, \fB%n\fR will be substituted with the service or environment variable name, +\fB%v\fR will be substituted with the environment variable value, and \fB%s\fR will be substituted +with a textual description of the new status (\fBstarted\fR, \fBstopped\fR or \fBfailed\fR for +services, \fBset\fR or \fBunset\fR for environment variables). A double percent sign (\fB%%\fR) is substituted with a single percent sign character. .\" .SH OPERATION diff --git a/doc/manpages/dinitctl.8.m4 b/doc/manpages/dinitctl.8.m4 index 3674a741..b51242c8 100644 --- a/doc/manpages/dinitctl.8.m4 +++ b/doc/manpages/dinitctl.8.m4 @@ -69,6 +69,9 @@ dinitctl \- control services supervised by Dinit [\fIoptions\fR] \fBsetenv\fR [\fIname\fR[=\fIvalue\fR] \fI...\fR] .HP .B dinitctl +[\fIoptions\fR] \fBunsetenv\fR [\fIname\fR \fI...\fR] +.HP +.B dinitctl [\fIoptions\fR] \fBcatlog\fR [\fB--clear\fR] \fIservice-name\fR .HP .B dinitctl @@ -322,6 +325,10 @@ called in. Any subsequently started or restarted service will have these environment variables available. This is particularly useful for user services that need access to session information. .TP +\fBunsetenv\fR +Unset one or more variables in the activation environment. +Any subsequently started or restarted service will have these environment variables unset. +.TP \fBcatlog\fR Show the contents of the log buffer for the named service. This is possible only if the log type of the service is set to \fBbuffer\fR. diff --git a/src/control.cc b/src/control.cc index 76d4a0e4..58985430 100644 --- a/src/control.cc +++ b/src/control.cc @@ -111,6 +111,8 @@ bool control_conn_t::process_packet() return process_setenv(); case cp_cmd::GETALLENV: return process_getallenv(); + case cp_cmd::LISTENENV: + return process_listenenv(); case cp_cmd::SETTRIGGER: return process_set_trigger(); case cp_cmd::CATLOG: @@ -1076,13 +1078,19 @@ bool control_conn_t::process_setenv() envVar = rbuf.extract_string(3, envvar_len); eq = envVar.find('='); - if (!eq || eq == envVar.npos) { - // Not found or at the beginning of the string + if (eq == envVar.npos) { + // Unset the env var + main_env.undefine_var(std::move(envVar), true); + } + else if (eq) { + // Regular set + main_env.set_var(std::move(envVar), true); + } + else { + // At the beginning of the string goto badreq; } - main_env.set_var(std::move(envVar)); - // Success response if (!queue_packet(okRep, 1)) return false; @@ -1141,6 +1149,17 @@ bool control_conn_t::process_getallenv() return true; } +bool control_conn_t::process_listenenv() +{ + // 1 byte packet type, nothing else + rbuf.consume(1); + + main_env.add_listener(this); + + char ack_rep[] = { (char)cp_rply::ACK }; + return queue_packet(ack_rep, 1); +} + bool control_conn_t::process_set_trigger() { // 1 byte packet type @@ -1468,6 +1487,28 @@ void control_conn_t::service_event(service_record *service, service_event_t even } } +void control_conn_t::environ_event(environment *env, std::string const &var_and_val, bool overridden) noexcept +{ + // packet type (byte) + packet length (byte) + flags byte + data size + data + // flags byte can be 1 or 0 for now, 1 if the var was overridden and 0 if fresh + constexpr int pktsize = 3 + sizeof(envvar_len_t); + envvar_len_t ln = var_and_val.size() + 1; + auto *ptr = var_and_val.data(); + + try { + std::vector pkt; + pkt.reserve(pktsize + ln); + pkt.push_back((char)cp_info::ENVEVENT); + pkt.push_back(pktsize); + pkt.push_back(overridden ? 1 : 0); + pkt.insert(pkt.end(), (char *)&ln, ((char *)&ln) + sizeof(envvar_len_t)); + pkt.insert(pkt.end(), ptr, ptr + ln); + queue_packet(std::move(pkt)); + } catch (std::bad_alloc &exc) { + do_oom_close(); + } +} + bool control_conn_t::queue_packet(const char *pkt, unsigned size) noexcept { bool was_empty = outbuf.empty(); @@ -1671,6 +1712,7 @@ control_conn_t::~control_conn_t() noexcept for (auto p : service_key_map) { p.first->remove_listener(this); } + main_env.remove_listener(this); active_control_conns--; } diff --git a/src/dinit-monitor.cc b/src/dinit-monitor.cc index f5109c4e..8f96f706 100644 --- a/src/dinit-monitor.cc +++ b/src/dinit-monitor.cc @@ -2,6 +2,7 @@ #include #include #include +#include #include #include @@ -32,10 +33,13 @@ struct stringview { static std::vector split_command(const char *cmd); static bool load_service(int socknum, cpbuffer_t &rbuffer, const char *name, handle_t *handle, service_state_t *state); +static void request_environ(int socknum, cpbuffer_t &rbuffer, uint16_t proto_version); +static size_t get_allenv(int socknum, cpbuffer_t &rbuffer); // dummy handler, so we can wait for children static void sigchld_handler(int) { } -void issue_command(const char* service_name, const char* event_str,std::vector &command_parts); +static void issue_command(const char* name, const char* value, const char* event, std::vector &command_parts, bool is_env = false); +static size_t read_var_and_issue(int socknum, cpbuffer_t &rbuffer, size_t dsz, const std::unordered_set &varset, std::string &enval, std::vector &command_parts, bool &issued, const char* str_set, const char* str_unset); int dinit_monitor_main(int argc, char **argv) { @@ -44,9 +48,13 @@ int dinit_monitor_main(int argc, char **argv) const char *control_socket_path = nullptr; bool user_dinit = (getuid() != 0); // communicate with user daemon bool issue_init = false; // request initial service state + bool use_environ = false; // listening on activation environment changes + bool exit_after = false; // exit after first issued command const char *str_started = "started"; const char *str_stopped = "stopped"; const char *str_failed = "failed"; + const char *str_set = "set"; + const char *str_unset = "unset"; const char *command_str = nullptr; std::vector services; @@ -61,6 +69,12 @@ int dinit_monitor_main(int argc, char **argv) std::cout << "Dinit version " << DINIT_VERSION << ".\n"; return 0; } + else if (strcmp(argv[i], "--exit") == 0 || strcmp(argv[i], "-e") == 0) { + exit_after = true; + } + else if (strcmp(argv[i], "--env") == 0 || strcmp(argv[i], "-E") == 0) { + use_environ = true; + } else if (strcmp(argv[i], "--system") == 0 || strcmp(argv[i], "-s") == 0) { user_dinit = false; } @@ -102,7 +116,23 @@ int dinit_monitor_main(int argc, char **argv) } str_failed = argv[i]; } - else if (strcmp(argv[i], "-c") == 0 || strcmp(argv[i], "--command")) { + else if (strcmp(argv[i], "--str-set") == 0) { + ++i; + if (i == argc) { + std::cerr << "dinit-monitor: --str-set should be followed by an argument\n"; + return 1; + } + str_set = argv[i]; + } + else if (strcmp(argv[i], "--str-unset") == 0) { + ++i; + if (i == argc) { + std::cerr << "dinit-monitor: --str-unset should be followed by an argument\n"; + return 1; + } + str_unset = argv[i]; + } + else if (strcmp(argv[i], "-c") == 0 || strcmp(argv[i], "--command") == 0) { ++i; if (i == argc) { std::cerr << "dinit-monitor: --command/-c should be followed by command\n"; @@ -120,10 +150,12 @@ int dinit_monitor_main(int argc, char **argv) std::cout << "dinit-monitor: monitor Dinit services\n" "\n" "Usage:\n" - " dinit-monitor [options] \n" + " dinit-monitor [options] \n" "\n" "Options:\n" " --help : show this help\n" + " -e, --exit : exit after the first issued command\n" + " -E, --env : monitor activation environment changes\n" " -s, --system : monitor system daemon (default if run as root)\n" " -u, --user : monitor user daemon\n" " -i, --initial : also execute command for initial service state\n" @@ -140,7 +172,7 @@ int dinit_monitor_main(int argc, char **argv) return 1; } - if (services.empty()) { + if (services.empty() && !use_environ) { std::cerr << "dinit-monitor: specify at least one service name\n"; return 1; } @@ -182,16 +214,24 @@ int dinit_monitor_main(int argc, char **argv) // Start by querying protocol version: cpbuffer_t rbuffer; - check_protocol_version(min_cp_version, max_cp_version, rbuffer, socknum); + uint16_t protocol_ver = check_protocol_version(min_cp_version, max_cp_version, rbuffer, socknum); // Load all services std::unordered_map service_map; + std::unordered_set environ_set; std::vector> service_init_state; + std::string env_value; for (const char *service_name : services) { handle_t hndl; service_state_t state; + + if (use_environ) { + environ_set.emplace(service_name); + continue; + } + if (!load_service(socknum, rbuffer, service_name, &hndl, &state)) { std::cerr << "dinit-monitor: cannot load service: " << service_name << "\n"; return 1; @@ -201,14 +241,35 @@ int dinit_monitor_main(int argc, char **argv) service_init_state.push_back(std::make_pair(service_name, state)); } - // Issue initial status commands if requested - if (issue_init) { + if (use_environ) { + // Request listening on environ events + request_environ(socknum, rbuffer, protocol_ver); + if (issue_init) { + // Get the whole block and see if it's already set + auto envsz = get_allenv(socknum, rbuffer); + while (envsz > 0) { + bool issued; + envsz = read_var_and_issue(socknum, rbuffer, envsz, environ_set, env_value, command_parts, issued, str_set, str_unset); + if (issued && exit_after) { + return 0; + } + } + } + } + else if (issue_init) { + // Issue initial status commands if requested for (auto state : service_init_state ) { if (state.second == service_state_t::STARTED) { - issue_command(state.first, str_started, command_parts); + issue_command(state.first, nullptr, str_started, command_parts); + if (exit_after) { + return 0; + } } else if (state.second == service_state_t::STOPPED) { - issue_command(state.first, str_stopped, command_parts); + issue_command(state.first, nullptr, str_stopped, command_parts); + if (exit_after) { + return 0; + } } } } @@ -221,7 +282,18 @@ int dinit_monitor_main(int argc, char **argv) int pktlen = (unsigned char) rbuffer[1]; fill_buffer_to(rbuffer, socknum, pktlen); - if (rbuffer[0] == (char)cp_info::SERVICEEVENT) { + if (use_environ && rbuffer[0] == (char)cp_info::ENVEVENT) { + envvar_len_t envln; + rbuffer.extract((char *) &envln, 3, sizeof(envln)); + rbuffer.consume(pktlen); + // this will return 0, we don't want to consume after this + bool issued; + pktlen = read_var_and_issue(socknum, rbuffer, envln, environ_set, env_value, command_parts, issued, str_set, str_unset); + if (issued && exit_after) { + return 0; + } + } + else if (!use_environ && rbuffer[0] == (char)cp_info::SERVICEEVENT) { handle_t ev_handle; rbuffer.extract((char *) &ev_handle, 2, sizeof(ev_handle)); service_event_t event = static_cast(rbuffer[2 + sizeof(ev_handle)]); @@ -241,7 +313,10 @@ int dinit_monitor_main(int argc, char **argv) else { goto consume_packet; } - issue_command(service_name, event_str, command_parts); + issue_command(service_name, nullptr, event_str, command_parts); + if (exit_after) { + return 0; + } } consume_packet: @@ -298,7 +373,7 @@ int dinit_monitor_main(int argc, char **argv) } -void issue_command(const char* service_name, const char* event_str, std::vector &command_parts) { +static void issue_command(const char* name, const char* value, const char* event, std::vector &command_parts, bool is_env) { std::vector final_cmd_parts; std::vector final_cmd_parts_cstr; @@ -314,10 +389,13 @@ void issue_command(const char* service_name, const char* event_str, std::vector< break; } if (cmd_part.str[i] == 'n') { - cmd_part_str.append(service_name); + cmd_part_str.append(name); + } + else if (cmd_part.str[i] == 'v') { + if (value) cmd_part_str.append(value); } else if (cmd_part.str[i] == 's') { - cmd_part_str.append(event_str); + cmd_part_str.append(event); } else { // invalid specifier, just output as is @@ -480,3 +558,103 @@ static bool load_service(int socknum, cpbuffer_t &rbuffer, const char *name, han return true; } + +static void request_environ(int socknum, cpbuffer_t &rbuffer, uint16_t proto_version) +{ + char c = (char)cp_cmd::LISTENENV; + + if (proto_version < 5) { + throw cp_old_server_exception(); + } + + write_all_x(socknum, &c, 1); + + wait_for_reply(rbuffer, socknum); + + cp_rply reply_pkt_h = (cp_rply)rbuffer[0]; + if (reply_pkt_h != cp_rply::ACK) { + throw dinit_protocol_error(); + } + rbuffer.consume(1); +} + +// get the whole environment block of the dinit instance in a way +// that leaves individual variables available for read, without +// the packet header; that allows read_var_and_issue to be used +static size_t get_allenv(int socknum, cpbuffer_t &rbuffer) +{ + char buf[2] = { (char)cp_cmd::GETALLENV, 0 }; + write_all_x(socknum, buf, 2); + + wait_for_reply(rbuffer, socknum); + + cp_rply reply_pkt_h = (cp_rply)rbuffer[0]; + if (reply_pkt_h != cp_rply::ALLENV) { + throw dinit_protocol_error(); + } + + // 1-byte packet header, then size_t + constexpr size_t allenv_hdr_size = 1 + sizeof(size_t); + rbuffer.fill_to(socknum, allenv_hdr_size); + + size_t dsize; + rbuffer.extract(&dsize, 1, sizeof(dsize)); + rbuffer.consume(allenv_hdr_size); + + return dsize; +} + +static bool issue_var(std::string &envar, const std::unordered_set &varset, std::vector &command_parts, + const char* str_set, const char* str_unset) +{ + auto eq = envar.find('='); + if (eq == envar.npos) { + /* unset */ + eq = envar.size(); + } + auto *sp = &envar[0]; + sp[eq] = '\0'; + if (varset.empty() || varset.find(sp) != varset.end()) { + if (eq == envar.size()) { + issue_command(sp, nullptr, str_unset, command_parts, true); + } + else { + issue_command(sp, &sp[eq + 1], str_set, command_parts, true); + } + return true; + } + return false; +} + +static size_t read_var_and_issue(int socknum, cpbuffer_t &rbuffer, size_t dsz, const std::unordered_set &varset, + std::string &enval, std::vector &command_parts, bool &issued, const char* str_set, const char *str_unset) +{ + enval.clear(); + issued = false; + while (dsz > 0) { + auto colen = rbuffer.get_contiguous_length(rbuffer.get_ptr(0)); + auto chlen = std::min((size_t)colen, dsz); + for (unsigned i = 0; i < chlen; ++i) { + if (rbuffer[i] != '\0') { + continue; + } + enval.append(rbuffer.get_ptr(0), rbuffer.get_ptr(0) + i); + rbuffer.consume(i + 1); + issued = issue_var(enval, varset, command_parts, str_set, str_unset); + return dsz - i - 1; + } + // copy what we have so far and fill some more + enval.append(rbuffer.get_ptr(0), rbuffer.get_ptr(0) + chlen); + rbuffer.consume(chlen); + dsz -= chlen; + if (dsz == 0) { + // didn't find null terminator, malformed + throw dinit_protocol_error(); + } + if (rbuffer.get_length() == 0) { + fill_some(rbuffer, socknum); + } + } + // unreachable + throw dinit_protocol_error(); +} diff --git a/src/dinitctl.cc b/src/dinitctl.cc index 45f60da5..504a0c50 100644 --- a/src/dinitctl.cc +++ b/src/dinitctl.cc @@ -58,7 +58,7 @@ static int add_remove_dependency(int socknum, cpbuffer_t &rbuffer, bool add, con const char *service_to, dependency_type dep_type, bool verbose); static int enable_disable_service(int socknum, cpbuffer_t &rbuffer, service_dir_opt &service_dir_opts, const char *from, const char *to, bool enable, bool verbose, uint16_t proto_version); -static int do_setenv(int socknum, cpbuffer_t &rbuffer, std::vector &env_names); +static int do_setenv(int socknum, cpbuffer_t &rbuffer, std::vector &env_names, bool unset); static int trigger_service(int socknum, cpbuffer_t &rbuffer, const char *service_name, bool trigger_value); static int cat_service_log(int socknum, cpbuffer_t &rbuffer, const char *service_name, bool do_clear); static int signal_send(int socknum, cpbuffer_t &rbuffer, const char *service_name, sig_num_t sig_num); @@ -82,6 +82,7 @@ enum class ctl_cmd { ENABLE_SERVICE, DISABLE_SERVICE, SETENV, + UNSETENV, SET_TRIGGER, UNSET_TRIGGER, CAT_LOG, @@ -277,6 +278,9 @@ int dinitctl_main(int argc, char **argv) else if (strcmp(argv[i], "setenv") == 0) { command = ctl_cmd::SETENV; } + else if (strcmp(argv[i], "unsetenv") == 0) { + command = ctl_cmd::UNSETENV; + } else if (strcmp(argv[i], "trigger") == 0) { command = ctl_cmd::SET_TRIGGER; } @@ -365,8 +369,8 @@ int dinitctl_main(int argc, char **argv) else if (command == ctl_cmd::ENABLE_SERVICE || command == ctl_cmd::DISABLE_SERVICE) { cmdline_error |= (to_service_name == nullptr); } - else if (command == ctl_cmd::SETENV) { - // Handle SETENV specially, since it needs arguments but they are not service names + else if (command == ctl_cmd::SETENV || command == ctl_cmd::UNSETENV) { + // Handle (UN)SETENV specially, since it needs arguments but they are not service names if (cmd_args.empty()) { cmdline_error = true; } @@ -460,6 +464,7 @@ int dinitctl_main(int argc, char **argv) " dinitctl [options] trigger \n" " dinitctl [options] untrigger \n" " dinitctl [options] setenv [name[=value] ...]\n" + " dinitctl [options] unsetenv [name ...]\n" " dinitctl [options] catlog \n" " dinitctl [options] signal \n" "\n" @@ -591,8 +596,8 @@ int dinitctl_main(int argc, char **argv) return enable_disable_service(socknum, rbuffer, service_dir_opts, service_name, to_service_name, command == ctl_cmd::ENABLE_SERVICE, verbose, daemon_protocol_ver); } - else if (command == ctl_cmd::SETENV) { - return do_setenv(socknum, rbuffer, cmd_args); + else if (command == ctl_cmd::SETENV || command == ctl_cmd::UNSETENV) { + return do_setenv(socknum, rbuffer, cmd_args, command == ctl_cmd::UNSETENV); } else if (command == ctl_cmd::SET_TRIGGER || command == ctl_cmd::UNSET_TRIGGER) { if (daemon_protocol_ver < 2) { @@ -2193,7 +2198,7 @@ static int enable_disable_service(int socknum, cpbuffer_t &rbuffer, service_dir_ return 0; } -static int do_setenv(int socknum, cpbuffer_t &rbuffer, std::vector &env_names) +static int do_setenv(int socknum, cpbuffer_t &rbuffer, std::vector &env_names, bool unset) { using namespace std; @@ -2210,14 +2215,19 @@ static int do_setenv(int socknum, cpbuffer_t &rbuffer, std::vector // either full var or name auto elen = strlen(envp); buf.append(envp, elen); - // if '=' not found, get value from environment - if (!memchr(envp, '=', elen)) { + // if '=' not found, get value from environment, except for unset + auto eq = memchr(envp, '=', elen); + if (!eq && !unset) { buf.push_back('='); auto *envv = getenv(envp); if (envv) { buf.append(envv); } } + else if (eq && unset) { + cerr << "dinitctl: environment variable '" << envp << "' must not contain the '=' sign." << endl; + return 1; + } envvar_len = buf.size() - hdr_len; // sanitize length early on if (buf.size() > cpbuffer_t::get_size()) { diff --git a/src/igr-tests/environ/setenv.sh b/src/igr-tests/environ/setenv.sh index 82ef040d..ca392d55 100755 --- a/src/igr-tests/environ/setenv.sh +++ b/src/igr-tests/environ/setenv.sh @@ -9,7 +9,7 @@ case "$1" in fi ;; setenv2) - if [ "$FOO" = "foo" ]; then + if [ "$FOO" = "foo" -a -z "$BAR" ]; then echo 2 >> "$OUTPUT" export BAR=bar "$DINITCTL" -p "$SOCKET" setenv BAR BAZ=baz @@ -17,6 +17,7 @@ case "$1" in ;; setenv3) "$DINITCTL" -p "$SOCKET" setenv FOO=foo + "$DINITCTL" -p "$SOCKET" unsetenv BAR echo 3 >> "$OUTPUT" ;; *) ;; diff --git a/src/igr-tests/igr-runner.cc b/src/igr-tests/igr-runner.cc index 662a7757..e9bb539e 100644 --- a/src/igr-tests/igr-runner.cc +++ b/src/igr-tests/igr-runner.cc @@ -175,6 +175,7 @@ void environ_test() igr_env_var_setup env_output("OUTPUT", output_file.c_str()); igr_env_var_setup env_socket("SOCKET", socket_path.c_str()); igr_env_var_setup env_dinitctl("DINITCTL", (dinit_bindir + "/dinitctl").c_str()); + igr_env_var_setup env_bar("BAR", "to_be_unset"); dinit_proc dinit_p; dinit_p.start("environ", {"-u", "-d", "sd", "-p", socket_path, "-q", "-e", "environment1", "checkenv"}); diff --git a/src/includes/control-cmds.h b/src/includes/control-cmds.h index cd9410c9..911fab3a 100644 --- a/src/includes/control-cmds.h +++ b/src/includes/control-cmds.h @@ -11,7 +11,8 @@ // 3 - dinit 0.17.1 (adds QUERYSERVICEDSCDIR) // 4 - dinit 0.18.0 (adds CLOSEHANDLE, GETALLENV) // 5 - dinit 0.19.1 (process status now represented as ([int]si_code + [int]si_status) rather than -// a single integer; SERVICEEVENT5 sent alongside SERVICEEVENT) +// a single integer; SERVICEEVENT5 sent alongside SERVICEEVENT; adds LISTENENV, ENVEVENT) +// (dinit 0.19.2 removed support for LISTENENV/ENVEVENT, they were added again in 0.19.3) // Requests: enum class cp_cmd : dinit_cptypes::cp_cmd_t { @@ -87,6 +88,9 @@ enum class cp_cmd : dinit_cptypes::cp_cmd_t { // Query status of an individual service (5+) SERVICESTATUS5 = 26, + + // Start listening to environment events + LISTENENV = 27, }; // Replies: @@ -175,6 +179,8 @@ enum class cp_info : dinit_cptypes::cp_info_t { SERVICEEVENT = 100, // Service event for protocol version 5+ - 4 byte handle, 1 byte event code, proc_status_t status SERVICEEVENT5 = 101, + // Environment event; 2 bytes length + env string + ENVEVENT = 102, }; #endif diff --git a/src/includes/control.h b/src/includes/control.h index 97ea5840..f78dc02d 100644 --- a/src/includes/control.h +++ b/src/includes/control.h @@ -12,6 +12,7 @@ #include #include +#include #include #include #include @@ -81,7 +82,7 @@ class control_conn_watcher : public eventloop_t::bidi_fd_watcher_impl #include +#include #include #include @@ -62,6 +63,14 @@ struct env_equal_name } }; +// Interface for listening to environment +class env_listener +{ + public: + + virtual void environ_event(environment *env, std::string const &name_and_val, bool overridden) noexcept = 0; +}; + class environment { // Whether to keep the parent environment, as a whole. Individual variables can still be @@ -81,6 +90,8 @@ class environment // set of variables modified or set: env_set set_vars; + std::unordered_set listeners; + string_view find_var_name(string_view var) { const char *var_ch; @@ -90,6 +101,13 @@ class environment return {var.data(), (size_t)(var_ch - var.data())}; } + void notify_listeners(std::string const &var_and_val, bool overridden) + { + for (auto l : listeners) { + l->environ_event(this, var_and_val, overridden); + } + } + public: environment() = default; @@ -253,16 +271,36 @@ class environment return build(env_names()); } - void set_var(std::string &&var_and_val) + void set_var(std::string &&var_and_val, bool notify = false) { string_view var_name = find_var_name(var_and_val); import_from_parent.erase(var_name); - undefine.erase(var_name); + auto n_removed = undefine.erase(var_name); + + bool in_sysenv = false; + if (notify && n_removed == 0) { + // workaround to avoid an allocation; when notify is true, + // we know for sure that the value is sanitized from before + // + // we don't check this when the variable was in undefine, + // as that means we were undefining it explicitly + auto name_size = var_name.size(); + var_and_val[name_size] = '\0'; + in_sysenv = !!bp_sys::getenv(var_and_val.c_str()); + var_and_val[name_size] = '='; + } auto insert_result = set_vars.insert(std::move(var_and_val)); + // if the variable was in sys environment, it is always overridden + bool overridden = in_sysenv; if (!insert_result.second) { *insert_result.first = var_and_val; + overridden = true; + } + + if (notify) { + notify_listeners(*insert_result.first, overridden); } } @@ -275,12 +313,24 @@ class environment } } - void undefine_var(std::string &&var_name) + void undefine_var(std::string &&var_name, bool notify = false) { import_from_parent.erase(var_name); - set_vars.erase(string_view(var_name)); + + auto n_removed = set_vars.erase(string_view(var_name)); + bool was_set = n_removed > 0; + if (notify && !was_set) { + // also track if we're undefining it from system environment + was_set = !!bp_sys::getenv(var_name.c_str()); + } if (keep_parent_env) { - undefine.insert(std::move(var_name)); + auto insert_result = undefine.insert(std::move(var_name)); + if (notify) { + notify_listeners(*insert_result.first, was_set); + } + } + else if (notify) { + notify_listeners(var_name, was_set); } } @@ -291,6 +341,16 @@ class environment undefine.clear(); set_vars.clear(); } + + void add_listener(env_listener * listener) + { + listeners.insert(listener); + } + + void remove_listener(env_listener * listener) noexcept + { + listeners.erase(listener); + } }; // Read and set environment variables from a file. May throw std::bad_alloc, std::system_error. diff --git a/src/tests/cptests/cptests.cc b/src/tests/cptests/cptests.cc index ce73d727..e0a7fa5d 100644 --- a/src/tests/cptests/cptests.cc +++ b/src/tests/cptests/cptests.cc @@ -1206,6 +1206,114 @@ void cptest_invalid() delete cc; } +void cptest_envevent() +{ + service_set sset; + int fd = bp_sys::allocfd(); + auto *cc = new control_conn_t(event_loop, &sset, fd); + + // Listen on environment: + std::vector cmd = { (char)cp_cmd::LISTENENV }; + + bp_sys::supply_read_data(fd, std::move(cmd)); + + event_loop.regd_bidi_watchers[fd]->read_ready(event_loop, fd); + + std::vector wdata; + bp_sys::extract_written_data(fd, wdata); + assert(wdata.size() == 1 /* ACK reply */); + + // Issue a setenv: + cmd = { (char)cp_cmd::SETENV }; + + const char *envn = "FOO=bar"; + envvar_len_t envl = strlen(envn); + cmd.insert(cmd.end(), (char *)&envl, ((char *)&envl) + sizeof(envl)); + cmd.insert(cmd.end(), envn, envn + envl); + + bp_sys::supply_read_data(fd, std::move(cmd)); + + event_loop.regd_bidi_watchers[fd]->read_ready(event_loop, fd); + + bp_sys::extract_written_data(fd, wdata); + + // Environment event + // packet type (1), packet length (1), flags (1), data (envl + 1) + assert(wdata[0] == (char)cp_info::ENVEVENT); + assert(wdata[1] == 3 + sizeof(envl)); + assert(wdata[2] == 0); + envl += 1; // null terminator + assert(memcmp(&envl, &wdata[3], sizeof(envl)) == 0); + assert(strcmp(&wdata[3 + sizeof(envl)], envn) == 0); + + // Override setenv + cmd = { (char)cp_cmd::SETENV }; + + envn = "FOO=baz"; + envl = strlen(envn); + cmd.insert(cmd.end(), (char *)&envl, ((char *)&envl) + sizeof(envl)); + cmd.insert(cmd.end(), envn, envn + envl); + + bp_sys::supply_read_data(fd, std::move(cmd)); + + event_loop.regd_bidi_watchers[fd]->read_ready(event_loop, fd); + + bp_sys::extract_written_data(fd, wdata); + + // Environment event + assert(wdata[0] == (char)cp_info::ENVEVENT); + assert(wdata[1] == 3 + sizeof(envl)); + assert(wdata[2] != 0); + envl += 1; // null terminator + assert(memcmp(&envl, &wdata[3], sizeof(envl)) == 0); + assert(strcmp(&wdata[3 + sizeof(envl)], envn) == 0); + + // Unset setenv + cmd = { (char)cp_cmd::SETENV }; + + envn = "FOO"; + envl = strlen(envn); + cmd.insert(cmd.end(), (char *)&envl, ((char *)&envl) + sizeof(envl)); + cmd.insert(cmd.end(), envn, envn + envl); + + bp_sys::supply_read_data(fd, std::move(cmd)); + + event_loop.regd_bidi_watchers[fd]->read_ready(event_loop, fd); + + bp_sys::extract_written_data(fd, wdata); + + assert(wdata[0] == (char)cp_info::ENVEVENT); + assert(wdata[1] == 3 + sizeof(envl)); + assert(wdata[2] != 0); + envl += 1; // null terminator + assert(memcmp(&envl, &wdata[3], sizeof(envl)) == 0); + assert(strcmp(&wdata[3 + sizeof(envl)], envn) == 0); + + // Unset setenv again to check override flag + cmd = { (char)cp_cmd::SETENV }; + + envn = "FOO"; + envl = strlen(envn); + cmd.insert(cmd.end(), (char *)&envl, ((char *)&envl) + sizeof(envl)); + cmd.insert(cmd.end(), envn, envn + envl); + + bp_sys::supply_read_data(fd, std::move(cmd)); + + event_loop.regd_bidi_watchers[fd]->read_ready(event_loop, fd); + + bp_sys::extract_written_data(fd, wdata); + + assert(wdata[0] == (char)cp_info::ENVEVENT); + assert(wdata[1] == 3 + sizeof(envl)); + assert(wdata[2] == 0); + envl += 1; // null terminator + assert(memcmp(&envl, &wdata[3], sizeof(envl)) == 0); + assert(strcmp(&wdata[3 + sizeof(envl)], envn) == 0); + + delete cc; +} + + #define RUN_TEST(name, spacing) \ std::cout << #name "..." spacing << std::flush; \ @@ -1234,5 +1342,6 @@ int main(int argc, char **argv) RUN_TEST(cptest_two_commands, " "); RUN_TEST(cptest_closehandle, " "); RUN_TEST(cptest_invalid, " "); + RUN_TEST(cptest_envevent, " "); return 0; } From 4adbe2a7f9f54b1cf0d6f9ff672604ce169fd9c2 Mon Sep 17 00:00:00 2001 From: Erica Z Date: Thu, 5 Dec 2024 11:23:31 +0100 Subject: [PATCH 26/51] fix dinit-monitor manpage --- doc/manpages/dinit-monitor.8.m4 | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/manpages/dinit-monitor.8.m4 b/doc/manpages/dinit-monitor.8.m4 index 6de49e31..fc422aa9 100644 --- a/doc/manpages/dinit-monitor.8.m4 +++ b/doc/manpages/dinit-monitor.8.m4 @@ -31,9 +31,11 @@ Display version and then exit. .TP \fB\-e\fR, \fB\-\-exit\fR Exit after the first command is executed, instead of waiting for more events. +.TP \fB\-E\fR, \fB\-\-env\fR Instead of monitoring the services, monitor changes in the global environment. If no environment variables are passed, all environment is monitored. +.TP \fB\-s\fR, \fB\-\-system\fR Control the system init process (this is the default when run as root). This option determines the default path to the control socket used to communicate with the \fBdinit\fR daemon From 0e76266246e788476746abc30a062195cadb126a Mon Sep 17 00:00:00 2001 From: Davin McCall Date: Sun, 8 Dec 2024 12:41:18 +1000 Subject: [PATCH 27/51] Update example services with preferred syntax --- doc/linux/services/auxfscheck | 10 +++++----- doc/linux/services/boot | 14 +++++++------- doc/linux/services/dbusd | 22 +++++++++++----------- doc/linux/services/dhcpcd | 4 ++-- doc/linux/services/filesystems | 9 ++++----- doc/linux/services/hwclock | 5 +++-- doc/linux/services/late-filesystems | 5 +++-- doc/linux/services/loginready | 8 ++++---- doc/linux/services/modules | 3 ++- doc/linux/services/rcboot | 3 ++- doc/linux/services/rootfscheck | 6 +++--- doc/linux/services/rootrw | 8 ++++---- doc/linux/services/single | 4 +++- doc/linux/services/sshd | 6 +++--- doc/linux/services/syslogd | 5 +++-- doc/linux/services/tty1 | 3 ++- doc/linux/services/tty2 | 3 ++- doc/linux/services/tty3 | 3 ++- doc/linux/services/tty4 | 3 ++- doc/linux/services/tty5 | 3 ++- doc/linux/services/tty6 | 3 ++- doc/linux/services/udev-settle | 6 ++++-- doc/linux/services/udev-trigger | 3 ++- doc/linux/services/udevd | 4 +++- 24 files changed, 80 insertions(+), 63 deletions(-) diff --git a/doc/linux/services/auxfscheck b/doc/linux/services/auxfscheck index 135871d6..89b5be1c 100644 --- a/doc/linux/services/auxfscheck +++ b/doc/linux/services/auxfscheck @@ -3,9 +3,9 @@ type = scripted command = /sbin/fsck -A -R -C -a restart = false -options = starts-on-console +options: starts-on-console -depends-on = early-filesystems -depends-on = udevd -depends-on = rootrw -waits-for = udev-settle +depends-on: early-filesystems +depends-on: udevd +depends-on: rootrw +waits-for: udev-settle diff --git a/doc/linux/services/boot b/doc/linux/services/boot index 7bf746cf..ee183319 100644 --- a/doc/linux/services/boot +++ b/doc/linux/services/boot @@ -3,12 +3,12 @@ type = internal # Each of these services starts a login prompt: -depends-ms = tty1 -depends-ms = tty2 -depends-ms = tty3 -depends-ms = tty4 -depends-ms = tty5 -depends-ms = tty6 +depends-ms: tty1 +depends-ms: tty2 +depends-ms: tty3 +depends-ms: tty4 +depends-ms: tty5 +depends-ms: tty6 # the boot.d directory contents determine other dependencies: -waits-for.d = boot.d +waits-for.d: boot.d diff --git a/doc/linux/services/dbusd b/doc/linux/services/dbusd index 885d0816..5c96d2fd 100644 --- a/doc/linux/services/dbusd +++ b/doc/linux/services/dbusd @@ -5,17 +5,17 @@ # can use the --print-address option for an effective readiness notification. # For non-socket-activated: -#type = process -#command = /usr/bin/dbus-daemon --system --nofork --nopidfile --print-address=4 -#depends-on = rcboot -#logfile = /var/log/dbus-daemon.log -#ready-notification = pipefd:4 -#smooth-recovery = yes - -# For socket-activation: type = process -command = /usr/bin/dbus-daemon --system --nofork --nopidfile -depends-on = rcboot +command = /usr/bin/dbus-daemon --system --nofork --nopidfile --print-address=4 logfile = /var/log/dbus-daemon.log +ready-notification = pipefd:4 smooth-recovery = yes -socket-listen = /var/run/dbus/system_bus_socket +depends-on: rcboot + +# For socket-activation: +#type = process +#command = /usr/bin/dbus-daemon --system --nofork --nopidfile +#socket-listen = /var/run/dbus/system_bus_socket +#logfile = /var/log/dbus-daemon.log +#smooth-recovery = yes +#depends-on: rcboot diff --git a/doc/linux/services/dhcpcd b/doc/linux/services/dhcpcd index 660dfc88..dd2af20b 100644 --- a/doc/linux/services/dhcpcd +++ b/doc/linux/services/dhcpcd @@ -5,5 +5,5 @@ command = /usr/sbin/dhcpcd -B -M --logfile /var/log/dhcpcd-service.log enp3s0 logfile = /var/log/dhcpcd.log restart = false -depends-on = rcboot -depends-on = netdev-enps3s0 +depends-on: rcboot +depends-on: netdev-enps3s0 diff --git a/doc/linux/services/filesystems b/doc/linux/services/filesystems index 1c2b7c51..c442e5fc 100644 --- a/doc/linux/services/filesystems +++ b/doc/linux/services/filesystems @@ -4,10 +4,9 @@ type = scripted command = /etc/dinit.d/filesystems.sh start restart = false logfile = /var/log/dinit-filesystems.log -options = start-interruptible start-timeout = 1200 # 20 minutes -depends-on = udevd -depends-on = rootrw -waits-for = auxfscheck -waits-for = udev-settle +depends-on: udevd +depends-on: rootrw +waits-for: auxfscheck +waits-for: udev-settle diff --git a/doc/linux/services/hwclock b/doc/linux/services/hwclock index 4ce05c7d..56684f2c 100644 --- a/doc/linux/services/hwclock +++ b/doc/linux/services/hwclock @@ -9,5 +9,6 @@ command = /sbin/hwclock --hctosys restart = false -depends-on = udevd -depends-on = early-filesystems +depends-on: udevd +waits-for: udev-settle +depends-on: early-filesystems diff --git a/doc/linux/services/late-filesystems b/doc/linux/services/late-filesystems index 0e165b27..1b458798 100644 --- a/doc/linux/services/late-filesystems +++ b/doc/linux/services/late-filesystems @@ -4,7 +4,8 @@ type = scripted command = /etc/dinit.d/late-filesystems.sh start restart = false logfile = /var/log/late-filesystems.log -options = start-interruptible start-timeout = 0 # unlimited -depends-on = rcboot +options: start-interruptible + +depends-on: rcboot diff --git a/doc/linux/services/loginready b/doc/linux/services/loginready index 4659ed48..02a07deb 100644 --- a/doc/linux/services/loginready +++ b/doc/linux/services/loginready @@ -4,7 +4,7 @@ type = internal restart = false options = runs-on-console -depends-on = rcboot -waits-for = dbusd -waits-for = udevd -waits-for = syslogd +depends-on: rcboot +waits-for: dbusd +waits-for: udevd +waits-for: syslogd diff --git a/doc/linux/services/modules b/doc/linux/services/modules index eeb7bc8c..c4486457 100644 --- a/doc/linux/services/modules +++ b/doc/linux/services/modules @@ -3,4 +3,5 @@ type = scripted command = /etc/dinit.d/modules.sh start restart = false -depends-on = early-filesystems + +depends-on: early-filesystems diff --git a/doc/linux/services/rcboot b/doc/linux/services/rcboot index 345e7f70..449bc774 100644 --- a/doc/linux/services/rcboot +++ b/doc/linux/services/rcboot @@ -5,4 +5,5 @@ command = /etc/dinit.d/rcboot.sh start stop-command = /etc/dinit.d/rcboot.sh stop logfile = /var/log/rcboot.log restart = false -depends-on = filesystems + +depends-on: filesystems diff --git a/doc/linux/services/rootfscheck b/doc/linux/services/rootfscheck index 48a8d6cc..69e95cb9 100644 --- a/doc/linux/services/rootfscheck +++ b/doc/linux/services/rootfscheck @@ -6,6 +6,6 @@ restart = false options = starts-on-console pass-cs-fd start-interruptible skippable start-timeout = 0 # unlimited -depends-on = early-filesystems -depends-on = udevd -waits-for = udev-trigger +depends-on: early-filesystems +depends-on: udevd +waits-for: udev-trigger diff --git a/doc/linux/services/rootrw b/doc/linux/services/rootrw index c4c3110c..bdbb74ce 100644 --- a/doc/linux/services/rootrw +++ b/doc/linux/services/rootrw @@ -6,7 +6,7 @@ restart = false options = starts-rwfs logfile = /run/rootrw.log -depends-on = early-filesystems -depends-on = udevd -waits-for = hwclock -waits-for = rootfscheck +depends-on: early-filesystems +depends-on: udevd +waits-for: hwclock +waits-for: rootfscheck diff --git a/doc/linux/services/single b/doc/linux/services/single index fa20b8c4..3fedd824 100644 --- a/doc/linux/services/single +++ b/doc/linux/services/single @@ -4,5 +4,7 @@ type = process command = /bin/sh restart = false -options = shares-console + chain-to = boot + +options: shares-console diff --git a/doc/linux/services/sshd b/doc/linux/services/sshd index 39212cec..66017857 100644 --- a/doc/linux/services/sshd +++ b/doc/linux/services/sshd @@ -2,6 +2,6 @@ type = process smooth-recovery = true command = /usr/sbin/sshd -D -depends-on = loginready -depends-on = rcboot -waits-for = syslogd +depends-on: loginready +depends-on: rcboot +waits-for: syslogd diff --git a/doc/linux/services/syslogd b/doc/linux/services/syslogd index 4c09f43b..9fc549a8 100644 --- a/doc/linux/services/syslogd +++ b/doc/linux/services/syslogd @@ -6,6 +6,7 @@ type = bgprocess smooth-recovery = true command = /usr/sbin/syslogd pid-file = /var/run/syslogd.pid -options = starts-log -depends-on = rcboot +options: starts-log + +depends-on: rcboot diff --git a/doc/linux/services/tty1 b/doc/linux/services/tty1 index 3c4b1a50..40e88e0b 100644 --- a/doc/linux/services/tty1 +++ b/doc/linux/services/tty1 @@ -1,8 +1,9 @@ type = process command = /sbin/agetty tty1 linux-c restart = true -depends-on = loginready termsignal = HUP smooth-recovery = true inittab-id = 1 inittab-line = tty1 + +depends-on: loginready diff --git a/doc/linux/services/tty2 b/doc/linux/services/tty2 index e9d8688c..0aa074a0 100644 --- a/doc/linux/services/tty2 +++ b/doc/linux/services/tty2 @@ -1,8 +1,9 @@ type = process command = /sbin/agetty tty2 linux-c restart = true -depends-on = loginready termsignal = HUP smooth-recovery = true inittab-id = 2 inittab-line = tty2 + +depends-on: loginready diff --git a/doc/linux/services/tty3 b/doc/linux/services/tty3 index 0095a444..2842a82d 100644 --- a/doc/linux/services/tty3 +++ b/doc/linux/services/tty3 @@ -1,8 +1,9 @@ type = process command = /sbin/agetty tty3 linux-c restart = true -depends-on = loginready termsignal = HUP smooth-recovery = true inittab-id = 3 inittab-line = tty3 + +depends-on: loginready diff --git a/doc/linux/services/tty4 b/doc/linux/services/tty4 index 059c0612..77645a9e 100644 --- a/doc/linux/services/tty4 +++ b/doc/linux/services/tty4 @@ -1,8 +1,9 @@ type = process command = /sbin/agetty tty4 linux-c restart = true -depends-on = loginready termsignal = HUP smooth-recovery = true inittab-id = 4 inittab-line = tty4 + +depends-on: loginready diff --git a/doc/linux/services/tty5 b/doc/linux/services/tty5 index 64c1babb..840918f7 100644 --- a/doc/linux/services/tty5 +++ b/doc/linux/services/tty5 @@ -1,8 +1,9 @@ type = process command = /sbin/agetty tty5 linux-c restart = true -depends-on = loginready termsignal = HUP smooth-recovery = true inittab-id = 5 inittab-line = tty5 + +depends-on: loginready diff --git a/doc/linux/services/tty6 b/doc/linux/services/tty6 index 07b74530..faaaca40 100644 --- a/doc/linux/services/tty6 +++ b/doc/linux/services/tty6 @@ -1,8 +1,9 @@ type = process command = /sbin/agetty tty6 linux-c restart = true -depends-on = loginready termsignal = HUP smooth-recovery = true inittab-id = 6 inittab-line = tty6 + +depends-on: loginready diff --git a/doc/linux/services/udev-settle b/doc/linux/services/udev-settle index f8e3ddfd..9083f130 100644 --- a/doc/linux/services/udev-settle +++ b/doc/linux/services/udev-settle @@ -1,5 +1,7 @@ type = scripted command = /bin/udevadm settle restart = false -waits-for = udevd -waits-for = udev-trigger +logfile = /run/udev-settle.log + +depends-on: udevd +depends-on: udev-trigger diff --git a/doc/linux/services/udev-trigger b/doc/linux/services/udev-trigger index ab13a0b7..7f600568 100644 --- a/doc/linux/services/udev-trigger +++ b/doc/linux/services/udev-trigger @@ -4,4 +4,5 @@ type = scripted command = /bin/udevadm trigger --action=add logfile = /run/udev-trigger.log restart = false -depends-on = udevd + +depends-on: udevd diff --git a/doc/linux/services/udevd b/doc/linux/services/udevd index 7cbc81ae..7cb78075 100644 --- a/doc/linux/services/udevd +++ b/doc/linux/services/udevd @@ -2,9 +2,11 @@ # there is no other way to get notification when the control socket is # ready. (The downside is that we cannot properly supervise the process # and restart it if it crashes). + type = scripted command = /sbin/udevd --daemon stop-command = /bin/udevadm control -e logfile = /run/udevd.log restart = false -depends-on = early-filesystems + +depends-on: early-filesystems From 2a12ccf241da72fab34cb76cdfd7cbca13c16b7a Mon Sep 17 00:00:00 2001 From: Davin McCall Date: Thu, 12 Dec 2024 21:37:43 +1000 Subject: [PATCH 28/51] Doc: change "minimal substitution" to "pre-load substitution" Earlier the variable substitution rules (for service settings) were changed so that settings where expansion occurs before the service environment is loaded can still expand variables set globally (in the dinit process). So, change "minimal substituion" to "pre-load substitution" and adjust the documentation accordingly. --- doc/manpages/dinit-service.5.m4 | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/doc/manpages/dinit-service.5.m4 b/doc/manpages/dinit-service.5.m4 index f614bc23..67b4b19c 100644 --- a/doc/manpages/dinit-service.5.m4 +++ b/doc/manpages/dinit-service.5.m4 @@ -285,7 +285,7 @@ This service depends on the named service. Starting this service will start the named service; the command to start this service will not be executed until the named service has started. If the named service stops then this service will also be stopped. -The \fIservice-name\fR is subject to minimal variable substitution +The \fIservice-name\fR is subject to pre-load variable substitution (see \fBVARIABLE SUBSTITUTION\fR). .TP \fBdepends\-ms\fR: \fIservice-name\fR @@ -295,7 +295,7 @@ named service has started, and will fail to start if the named service does not start. Once the named (dependent) service reaches the started state, however, the dependency may stop without affecting the dependent service. -The name is likewise subject to minimal variable substitution. +The name is likewise subject to pre-load variable substitution. .TP \fBwaits\-for\fR: \fIservice-name\fR When this service is started, wait for the named service to finish starting @@ -303,7 +303,7 @@ When this service is started, wait for the named service to finish starting Starting this service will automatically start the named service. If the named service fails to start, this service will start as usual (subject to other dependencies being met). -The name is likewise subject to minimal variable substitution. +The name is likewise subject to pre-load variable substitution. .TP \fBdepends\-on.d\fR: \fIdirectory-path\fR For each file name in \fIdirectory-path\fR which does not begin with a dot, @@ -332,7 +332,7 @@ starting this service will not cause it to start (nor wait for it in that case). It does not by itself cause the named service to be loaded (if loaded later, the "after" relationship will be enforced from that point). .TP -The name is subject to minimal variable substitution. +The name is subject to pre-load variable substitution. .TP \fBbefore\fR: \fIservice-name\fR When starting the named service, if this service is also starting, wait for this service @@ -341,7 +341,7 @@ an \fBafter\fR relationship to this service from the named service. However, it does not by itself cause the named service to be loaded (if loaded later, the "before" relationship will be enforced from that point). .TP -The name is subject to minimal variable substitution. +The name is subject to pre-load variable substitution. .TP \fBchain\-to\fR = \fIservice-name\fR When this service terminates (i.e. starts successfully, and then stops of its @@ -364,7 +364,7 @@ abnormally or with an exit status indicating an error. However, if the \fBalways-chain\fR option is set the chain is started regardless of the reason and the status of this service termination. .IP -The name is subject to minimal variable substitution. +The name is subject to pre-load variable substitution. .TP \fBsocket\-listen\fR = \fIsocket-path\fR Pre-open a socket for the service and pass it to the service using the @@ -774,11 +774,11 @@ Using environment variable values in service commands and parameters can be used provide easily-adjustable service configuration, but is not ideal for this purpose and alternatives should be considered. .LP -In dependency fields, including \fIbefore\fR and similar, minimal version of variable -substitution may happen. -Only the service argument may be substituted, as the actual environment is not available -at this point. -The full syntax is still supported. +In dependency fields, including \fIdepends\-on\fR as well as \fIbefore\fR/\fIafter\fR and similar, +variable substitution may happen before the service environment is loaded. +This "pre-load" expansion can substitute service arguments and environment variables set within +dinit only; any service-specific variables that will be loaded from file (as specified using \fBenv\-file\fR) +are not available. .\" .SS META-COMMANDS .\" @@ -791,6 +791,8 @@ The following commands are available: \fB@include\fR \fIpath\fR Include the contents of another file, specified via its full path. If the specified file does not exist, an error is produced. +The \fIpath\fR is subject to pre-load variable substitution +(see \fBVARIABLE SUBSTITUTION\fR). .TP \fB@include\-opt\fR \fIpath\fR As for \fB@include\fR, but produces no error if the named file does not exist. From ce0a6dee6b7a113c77344dc1739018fb7abf8794 Mon Sep 17 00:00:00 2001 From: Davin McCall Date: Thu, 12 Dec 2024 22:10:49 +1000 Subject: [PATCH 29/51] Doc: rework documentation for service arguments a little --- doc/manpages/dinit-service.5.m4 | 37 ++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/doc/manpages/dinit-service.5.m4 b/doc/manpages/dinit-service.5.m4 index 67b4b19c..68854e6c 100644 --- a/doc/manpages/dinit-service.5.m4 +++ b/doc/manpages/dinit-service.5.m4 @@ -12,32 +12,35 @@ Dinit service description files .SH DESCRIPTION .\" The service description files for \fBDinit\fR each describe a service. The name -of the file corresponds to the name of the service it describes, minus its argument. +of the file corresponds to the name of the service it describes (minus its argument; +see \fBSERVICE ARGUMENTS\fR). .LP -Service description files specify the various attributes of a service. A -service description file is named after the service it represents (without -its argument), and is a plain-text file with simple key-value format. +Service description files specify the various attributes of a service. +A service description file is a plain-text file with simple key-value format. The description files are located in a service description directory; See \fBdinit\fR(8) for more details of the default service description directories, and how and when service descriptions are loaded. .LP -The full name of the service includes its argument, such as \fIservice@argument\fR. -The argument is optional, so you can also invoke just \fIservice\fR. -Each instance of a service, i.e. with different arguments, is separate, including loading. -This means every time you invoke the service with a different argument, it is loaded -separately. -Empty argument is not the same as missing argument, as this affects variable -substitution (see \fBVARIABLE SUBSTITUTION\fR). -.LP -All services have a \fItype\fR and a set of \fIdependencies\fR. These are discussed -in the following subsections. The type, dependencies, and other attributes are -specified via property settings, the format of which are documented in the -\fBSERVICE PROPERTIES\fR subsection, which also lists the available properties. +All services have a \fItype\fR and a set of \fIdependencies\fR. +These are discussed in the following subsections. +The type, dependencies, and other attributes are specified via property settings, the format of +which are documented in the \fBSERVICE PROPERTIES\fR subsection, which also lists the available +properties. .LP In addition to service properties, some meta-commands can be used within service description files. See the \fBMETA-COMMANDS\fR subsection for more information. .\" +.SS SERVICE ARGUMENTS +.\" +A service description may act as a template for multiple related services. +The full name of a service can include an argument, following the base name of the service +suffixed by an 'at' symbol (\fB@\fR), in the form \fIbase-name@argument\fR. +The argument value may be substituted into various service setting values by using +a '\fB$1\fR' marker in the value specified in the service description (see \fBVARIABLE SUBSTITUTION\fR). +The full name (base name together with argument) uniquely identifies a service instance, and each +instance is loaded separately, remaining independent of other instances. +.\" .SS SERVICE TYPES .\" There are five basic types of service: @@ -774,7 +777,7 @@ Using environment variable values in service commands and parameters can be used provide easily-adjustable service configuration, but is not ideal for this purpose and alternatives should be considered. .LP -In dependency fields, including \fIdepends\-on\fR as well as \fIbefore\fR/\fIafter\fR and similar, +In dependency fields, including \fBdepends\-on\fR as well as \fBbefore\fR/\fBafter\fR and similar, variable substitution may happen before the service environment is loaded. This "pre-load" expansion can substitute service arguments and environment variables set within dinit only; any service-specific variables that will be loaded from file (as specified using \fBenv\-file\fR) From b9a0f1a278ca5d83af319e201cb1245fc3053790 Mon Sep 17 00:00:00 2001 From: Davin McCall Date: Tue, 7 Jan 2025 22:31:49 +1000 Subject: [PATCH 30/51] Version 0.19.3 --- NEWS | 10 ++++++++++ README.md | 2 +- build/version.conf | 6 +++--- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/NEWS b/NEWS index 9aa8d6c4..6bf99ca5 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,13 @@ +== Version 0.19.3 (beta release #7) + +Release 0.19.1 mistakenly included in-development features (mainly, service arguments) that were +not intended to be included in a bug-fix release. Release 0.19.2 did not include those features. +Since distributions may be relying on the features, this release adds them back. There are a +small number of documentation fixes and improvements also. + +Thanks to current sponsors: Paweł Zmarzły (pzmarzly), Wesley Moore (wezm), +M. Herdiansyah (konimex), Coleman McFarland (dontlaugh), q66, saolof, and private sponsors. + == Version 0.19.2 (beta release #6) This beta release contains minor bugfixes compared to the prior release. It does not contain diff --git a/README.md b/README.md index 77e0b05d..35b6d2a9 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # Dinit -v0.19.2 +v0.19.3 --- ![Dinit logo](doc/dinit-logo.png) diff --git a/build/version.conf b/build/version.conf index 04aec1f0..3bd755ba 100644 --- a/build/version.conf +++ b/build/version.conf @@ -1,4 +1,4 @@ # Included from Makefiles. -VERSION=0.19.2 -MONTH=December -YEAR=2024 +VERSION=0.19.3 +MONTH=January +YEAR=2025 From fa2660592e75140a9ac7e14258da3006ba5408ce Mon Sep 17 00:00:00 2001 From: Davin McCall Date: Wed, 8 Jan 2025 08:20:58 +1000 Subject: [PATCH 31/51] Bump version (to 0.19.4pre, for now) --- NEWS | 6 +++++- README.md | 2 +- build/version.conf | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/NEWS b/NEWS index 6bf99ca5..c7b53a32 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,7 @@ +== Version ??? + +TBA + == Version 0.19.3 (beta release #7) Release 0.19.1 mistakenly included in-development features (mainly, service arguments) that were @@ -8,7 +12,7 @@ small number of documentation fixes and improvements also. Thanks to current sponsors: Paweł Zmarzły (pzmarzly), Wesley Moore (wezm), M. Herdiansyah (konimex), Coleman McFarland (dontlaugh), q66, saolof, and private sponsors. -== Version 0.19.2 (beta release #6) +== Version 0.19.2 This beta release contains minor bugfixes compared to the prior release. It does not contain various feature changes that be released in version 0.20.0 in due course. diff --git a/README.md b/README.md index 35b6d2a9..db275426 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # Dinit -v0.19.3 +v0.19.4pre --- ![Dinit logo](doc/dinit-logo.png) diff --git a/build/version.conf b/build/version.conf index 3bd755ba..98bd1a9c 100644 --- a/build/version.conf +++ b/build/version.conf @@ -1,4 +1,4 @@ # Included from Makefiles. -VERSION=0.19.3 +VERSION=0.19.4pre MONTH=January YEAR=2025 From c3aaaea7e331b8c8ba74cdbb7c47a58cd6f0ff0f Mon Sep 17 00:00:00 2001 From: Emile 'iMil' Heitor Date: Tue, 28 Jan 2025 06:38:41 +0000 Subject: [PATCH 32/51] configure: add NetBSD as a supported platform and modified PLATFORM test to "case in" --- configure | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/configure b/configure index 365c32a8..c50fb3c4 100755 --- a/configure +++ b/configure @@ -328,10 +328,12 @@ fi [ -z "${STRIPOPTS+IS_SET}" ] && STRIPOPTS="-s" ## Verify PLATFORM value -if [ "$PLATFORM" != "Linux" ] && [ "$PLATFORM" != "FreeBSD" ] && \ -[ "$PLATFORM" != "OpenBSD" ] && [ "$PLATFORM" != "Darwin" ]; then - warning "$PLATFORM platform is unknown!" "Known Platforms are: Linux, FreeBSD, OpenBSD, Darwin" -fi +case "$PLATFORM" in + Linux|FreeBSD|NetBSD|OpenBSD|Darwin) ;; + *) warning "$PLATFORM platform is unknown!" \ + "Known Platforms are: Linux, FreeBSD, NetBSD, OpenBSD, Darwin" + ;; +esac ## Create testfile.cc to test c++ compiler echo "int main(int argc, char **argv) { return 0; }" > testfile.cc || error Can\'t create temporary file From aa4f2a655dfa71196a79a83d48f0b1ab2d1b5b2d Mon Sep 17 00:00:00 2001 From: Emile 'iMil' Heitor Date: Tue, 28 Jan 2025 10:25:48 +0000 Subject: [PATCH 33/51] Add shutdown support for NetBSD --- src/shutdown.cc | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/shutdown.cc b/src/shutdown.cc index 2e4ea915..6a6ab0ce 100644 --- a/src/shutdown.cc +++ b/src/shutdown.cc @@ -496,7 +496,11 @@ void do_system_shutdown(shutdown_type_t shutdown_type) sub_buf.append("Issuing shutdown via kernel...\n"); loop.poll(); // give message a chance to get to console +#ifdef __NetBSD__ + reboot(reboot_type, NULL); +#else reboot(reboot_type); +#endif } // Watcher for subprocess output. @@ -637,7 +641,11 @@ static loop_t::child_proc_watcher::proc_status_t run_process(const char * prog_a static void unmount_disks(loop_t &loop, subproc_buffer &sub_buf) { try { +#ifdef __NetBSD__ + const char * unmount_args[] = { "/sbin/umount", "-a", nullptr }; +#else const char * unmount_args[] = { "/bin/umount", "-a", "-r", nullptr }; +#endif run_process(unmount_args, loop, sub_buf); } catch (std::exception &e) { @@ -650,7 +658,11 @@ static void unmount_disks(loop_t &loop, subproc_buffer &sub_buf) static void swap_off(loop_t &loop, subproc_buffer &sub_buf) { try { +#ifdef __NetBSD__ + const char * swapoff_args[] = { "/sbin/swapctl", "-U", nullptr }; +#else const char * swapoff_args[] = { "/sbin/swapoff", "-a", nullptr }; +#endif run_process(swapoff_args, loop, sub_buf); } catch (std::exception &e) { From da7f9a3061c9bce6023128e5b1686d4fc58b3bc6 Mon Sep 17 00:00:00 2001 From: Mobin Aydinfar Date: Wed, 5 Feb 2025 23:28:21 +0330 Subject: [PATCH 34/51] fuzzer: Fix build when building with libcap Signed-off-by: Mobin Aydinfar --- src/tests/cptests/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests/cptests/Makefile b/src/tests/cptests/Makefile index 220614f0..35ebda14 100644 --- a/src/tests/cptests/Makefile +++ b/src/tests/cptests/Makefile @@ -38,7 +38,7 @@ fuzz_objects = $(foreach obj,$(parent_objs),fuzz-$(obj)) $(MAKE) -C ../../../build all fuzz: ../../../build/includes/mconfig.h fuzz.cc $(fuzz_parent_test_objects) $(fuzz_objects) - clang++ -std=c++11 -g -O1 -I../test-includes -I../../../dasynq/include -I../../../build/includes/ -I../../includes -fsanitize=fuzzer,address,undefined,leak fuzz.cc $(fuzz_parent_test_objects) $(fuzz_objects) -o fuzz + clang++ -std=c++11 -g -O1 -I../test-includes -I../../../dasynq/include -I../../../build/includes/ -I../../includes -fsanitize=fuzzer,address,undefined,leak fuzz.cc $(fuzz_parent_test_objects) $(fuzz_objects) $(LDFLAGS_LIBCAP) -o fuzz $(fuzz_parent_test_objects): fuzz-%.o: ../%.cc clang -O1 -fsanitize=address,undefined,fuzzer-no-link,leak -MMD -MP -I../test-includes -I../../../dasynq/include -I../../../build/includes/ -I../../includes -c $< -o $@ From 8355c509a3f18227bc263acf3a39d6d0923b6561 Mon Sep 17 00:00:00 2001 From: Davin McCall Date: Sun, 9 Feb 2025 19:55:29 +1000 Subject: [PATCH 35/51] Fix glitch where dinitctl status incorrectly shows "No error" --- src/dinitctl.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/dinitctl.cc b/src/dinitctl.cc index 504a0c50..9c098bc6 100644 --- a/src/dinitctl.cc +++ b/src/dinitctl.cc @@ -1640,6 +1640,10 @@ static int service_status(int socknum, cpbuffer_t &rbuffer, const char *service_ case stopped_reason_t::EXECFAILED: uint16_t launch_stage; rbuffer.extract((char *)&launch_stage, 4, 2); + if (exit_status == 0) { + // (Protocol version 5+) + exit_status = exit_si_code; + } cout << " (could not be launched)\n"; cout << " Stage: " << exec_stage_descriptions[launch_stage] << "\n"; cout << " Error: " << strerror(exit_status); From 5747b8fa25ba074907b88d43d6045dfdfb8b59ae Mon Sep 17 00:00:00 2001 From: Mobin Aydinfar Date: Mon, 10 Feb 2025 17:22:46 +0330 Subject: [PATCH 36/51] meson: Add missing service arguments related tests Signed-off-by: Mobin Aydinfar --- src/igr-tests/meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/igr-tests/meson.build b/src/igr-tests/meson.build index 350acf86..8d1ff0e2 100644 --- a/src/igr-tests/meson.build +++ b/src/igr-tests/meson.build @@ -17,7 +17,7 @@ tests = [ 'check-basic', 'check-cycle', 'check-cycle2', 'check-lint', 'reload1', 'reload2', 'no-command-error', 'add-rm-dep', 'var-subst', 'svc-start-fail', 'dep-not-found', 'pseudo-cycle', 'before-after', 'before-after2', 'log-via-pipe', 'catlog', - 'offline-enable', 'xdg-config', 'cycles' + 'offline-enable', 'xdg-config', 'cycles', 'svc-arg' ] foreach test: tests From 31c32122baf9e5b546df7ffe28f273357507d08f Mon Sep 17 00:00:00 2001 From: Mobin Aydinfar Date: Thu, 13 Feb 2025 17:09:21 +0330 Subject: [PATCH 37/51] CI: Update freebsd-14-0 to freebsd-14-2 FreeBSD 14.0-RELEASE is EOF[1] and also removed from CirrusCI. This commit fixes recent CI failure. [1]: https://www.freebsd.org/security/unsupported/ Signed-off-by: Mobin Aydinfar --- .cirrus.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.cirrus.yml b/.cirrus.yml index d1c85894..f66ea045 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -21,7 +21,7 @@ Dinit with hardening CI_task: TEST_LDFLAGS: # ASLR breaks -fsanitize=address,undefined freebsd_instance: - image_family: freebsd-14-0 + image_family: freebsd-14-2 Getting depends_script: pkg update && ASSUME_ALWAYS_YES=YES pkg install gmake m4 file llvm15 Configure_script: ./configure From 90ce6fed1ff2f91bb3a497e5a49519e83bca153c Mon Sep 17 00:00:00 2001 From: Mobin Aydinfar Date: Sat, 8 Mar 2025 16:23:00 +0330 Subject: [PATCH 38/51] service-constants: Add error description for SET_CAPS and SET_PRIO execution stages This will avoid a null dereference when logging an error in those stages. Signed-off-by: Mobin Aydinfar --- src/includes/service-constants.h | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/includes/service-constants.h b/src/includes/service-constants.h index 2787c15e..99ced0da 100644 --- a/src/includes/service-constants.h +++ b/src/includes/service-constants.h @@ -95,8 +95,18 @@ const char * const exec_stage_descriptions[/* static_cast(exec_stage::DO_EX "setting user/group ID", // SET_UIDGID "opening log file", // OPEN_LOGFILE // SPARE1 used - nullptr, // SPARE2 - nullptr, // SPARE3 + #if SUPPORT_CAPABILITIES + "setting capabilities", // SET_CAPS + #else + "", // SET_CAPS (placeholder) + #endif + // SPARE2 used + #if SUPPORT_IOPRIO + "setting I/O priority", // SET_PRIO + #else + "", // SET_PRIO (placeholder) + #endif + // SPARE3 used nullptr, // SPARE4 nullptr, // SPARE5 nullptr, // SPARE6 From d1849759a2ffa4532eaac094231925e6f513ec98 Mon Sep 17 00:00:00 2001 From: Mobin Aydinfar Date: Wed, 26 Mar 2025 11:44:02 +0330 Subject: [PATCH 39/51] igr-runner: Fix inconsistent trailing slash for igr_input_basedir Don't add trailing slash to igr_input_basedir and not expect tools to provide directories with trailing slash, instead use slash when it's appropriate. Signed-off-by: Mobin Aydinfar --- src/igr-tests/igr-runner.cc | 2 +- src/igr-tests/igr.h | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/igr-tests/igr-runner.cc b/src/igr-tests/igr-runner.cc index e9bb539e..7a22e976 100644 --- a/src/igr-tests/igr-runner.cc +++ b/src/igr-tests/igr-runner.cc @@ -72,7 +72,7 @@ int main(int argc, char **argv) igr_output_basedir = env_igr_output_base; } - igr_input_basedir = get_full_cwd() + "/"; + igr_input_basedir = get_full_cwd(); char *env_igr_input_basedir = getenv("IGR_INPUT_BASE"); if (env_igr_input_basedir != nullptr) { igr_input_basedir = env_igr_input_basedir; diff --git a/src/igr-tests/igr.h b/src/igr-tests/igr.h index 2b3e9abc..27b2bd9c 100644 --- a/src/igr-tests/igr.h +++ b/src/igr-tests/igr.h @@ -297,7 +297,7 @@ class dinit_proc : public igr_proc args.insert(args.begin(), "--ready-fd"); } - igr_proc::start((igr_input_basedir + wdir).c_str(), (dinit_bindir + "/dinit").c_str(), args); + igr_proc::start((igr_input_basedir + "/" + wdir).c_str(), (dinit_bindir + "/dinit").c_str(), args); if (with_ready_wait) { while (ready_pipe_ptr->get_output().empty()) { @@ -316,7 +316,7 @@ class dinitctl_proc : public igr_proc void start(const char *wdir, std::vector args = {}) { - igr_proc::start((igr_input_basedir + wdir).c_str(), (dinit_bindir + "/dinitctl").c_str(), args); + igr_proc::start((igr_input_basedir + "/" + wdir).c_str(), (dinit_bindir + "/dinitctl").c_str(), args); } }; @@ -329,7 +329,7 @@ class dinitcheck_proc : public igr_proc void start(const char *wdir, std::vector args = {}) { - igr_proc::start((igr_input_basedir + wdir).c_str(), (dinit_bindir + "/dinitcheck").c_str(), args, + igr_proc::start((igr_input_basedir + "/" + wdir).c_str(), (dinit_bindir + "/dinitcheck").c_str(), args, true /* combine stdout/err */); } }; From 7bd42afba8be149db1d5e7e467344974896de5ba Mon Sep 17 00:00:00 2001 From: Davin McCall Date: Sun, 6 Apr 2025 22:23:11 +1000 Subject: [PATCH 40/51] Improve documentation for pre-load variable expansion --- doc/manpages/dinit-service.5.m4 | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/doc/manpages/dinit-service.5.m4 b/doc/manpages/dinit-service.5.m4 index 68854e6c..f1c1d7b8 100644 --- a/doc/manpages/dinit-service.5.m4 +++ b/doc/manpages/dinit-service.5.m4 @@ -335,7 +335,7 @@ starting this service will not cause it to start (nor wait for it in that case). It does not by itself cause the named service to be loaded (if loaded later, the "after" relationship will be enforced from that point). .TP -The name is subject to pre-load variable substitution. +The name is subject to pre-load variable substitution (see \fBVARIABLE SUBSTITUTION\fR). .TP \fBbefore\fR: \fIservice-name\fR When starting the named service, if this service is also starting, wait for this service @@ -344,7 +344,7 @@ an \fBafter\fR relationship to this service from the named service. However, it does not by itself cause the named service to be loaded (if loaded later, the "before" relationship will be enforced from that point). .TP -The name is subject to pre-load variable substitution. +The name is subject to pre-load variable substitution (see \fBVARIABLE SUBSTITUTION\fR). .TP \fBchain\-to\fR = \fIservice-name\fR When this service terminates (i.e. starts successfully, and then stops of its @@ -367,7 +367,7 @@ abnormally or with an exit status indicating an error. However, if the \fBalways-chain\fR option is set the chain is started regardless of the reason and the status of this service termination. .IP -The name is subject to pre-load variable substitution. +The name is subject to pre-load variable substitution (see \fBVARIABLE SUBSTITUTION\fR). .TP \fBsocket\-listen\fR = \fIsocket-path\fR Pre-open a socket for the service and pass it to the service using the @@ -771,17 +771,19 @@ the process environment of \fBdinit\fR (which is established on launch by the pr parent, amended by loading the environment file (if any) as specified in \fBdinit\fR(8), and further amended via \fBdinitctl setenv\fR commands or equivalent). .LP -Note that since variable substitution is performed on service load, the values seen by a service process may differ from those -used for substitution, if they have been changed in the meantime. +Note that since variable substitution is performed on service load, the values seen by a service +process may differ from those used for substitution, if they have been changed in the meantime. Using environment variable values in service commands and parameters can be used as means to provide easily-adjustable service configuration, but is not ideal for this purpose and alternatives should be considered. .LP -In dependency fields, including \fBdepends\-on\fR as well as \fBbefore\fR/\fBafter\fR and similar, -variable substitution may happen before the service environment is loaded. -This "pre-load" expansion can substitute service arguments and environment variables set within -dinit only; any service-specific variables that will be loaded from file (as specified using \fBenv\-file\fR) -are not available. +A "pre-load" variable substitution is performed for certain service properties (as documented), +including \fBdepends\-on\fR as well as \fBbefore\fR/\fBafter\fR and similar, instead of the usual +(post-load) substitution. +This form of substitution is performed before the service environment is loaded. +It can substitute service arguments and environment variables set within \fBdinit\fR only; any +service-specific variables that will be loaded from file (as specified using \fBenv\-file\fR) are +not available. .\" .SS META-COMMANDS .\" From f93608fe20e3eb9d486b576ea17da4aff3f62b2a Mon Sep 17 00:00:00 2001 From: Davin McCall Date: Tue, 8 Apr 2025 16:16:33 +1000 Subject: [PATCH 41/51] Improve the comment for cap_iab_wrapper and move it to types section --- src/includes/dinit-util.h | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/src/includes/dinit-util.h b/src/includes/dinit-util.h index 9c1b8254..cf1a0375 100644 --- a/src/includes/dinit-util.h +++ b/src/includes/dinit-util.h @@ -108,6 +108,42 @@ class string_view } }; +#if SUPPORT_CAPABILITIES +// A thin wrapper around the cap_iab_t structure to manage ownership (supports move) +struct cap_iab_wrapper { + cap_iab_wrapper() {} + cap_iab_wrapper(std::string const &str) noexcept { + if (str.empty()) return; + // this may end up being nullptr + // throwing from constructors is bad, so always check .get() afterwards + iab = cap_iab_from_text(str.c_str()); + } + + cap_iab_wrapper(cap_iab_wrapper const &) = delete; + cap_iab_wrapper(cap_iab_wrapper &&v) noexcept: iab(v.iab) { + v.iab = nullptr; + } + + cap_iab_wrapper &operator=(cap_iab_wrapper const &) = delete; + cap_iab_wrapper &operator=(cap_iab_wrapper &&v) noexcept { + iab = v.iab; + v.iab = nullptr; + return *this; + } + + ~cap_iab_wrapper() noexcept { + if (iab) cap_free(iab); + } + + cap_iab_t get() const noexcept { + return iab; + } + +private: + cap_iab_t iab = nullptr; +}; +#endif + // Complete read - read the specified size until end-of-file or error; continue read if // interrupted by signal. inline ssize_t complete_read(int fd, void * buf, size_t n) From b35a0e39547af1a93d3560c38e46fcf935a89c99 Mon Sep 17 00:00:00 2001 From: Davin McCall Date: Wed, 9 Apr 2025 22:53:06 +1000 Subject: [PATCH 42/51] cpbuffer: contiguous length when buffer empty is always 0 --- src/includes/cpbuffer.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/includes/cpbuffer.h b/src/includes/cpbuffer.h index 9e0352f4..f1181864 100644 --- a/src/includes/cpbuffer.h +++ b/src/includes/cpbuffer.h @@ -45,6 +45,8 @@ template class cpbuffer unsigned get_contiguous_length(char *ptr) { + if (length == 0) return 0; + unsigned eidx = cur_idx + length; if (eidx >= SIZE) eidx -= SIZE; From b750347fdf18ab21874bc44dfdd85db275ff03cb Mon Sep 17 00:00:00 2001 From: Mahno Date: Fri, 11 Apr 2025 21:36:09 +0800 Subject: [PATCH 43/51] fix SUPPORT_CGROUPS for non-cgroups systems --- src/dinit.cc | 14 +++++++------- src/run-child-proc.cc | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/dinit.cc b/src/dinit.cc index d2d0510d..52f667b0 100644 --- a/src/dinit.cc +++ b/src/dinit.cc @@ -67,7 +67,7 @@ static void flush_log() noexcept; static void control_socket_cb(eventloop_t *loop, int fd) noexcept; -#ifdef SUPPORT_CGROUPS +#if SUPPORT_CGROUPS static void find_cgroup_path() noexcept; #endif @@ -101,7 +101,7 @@ static bool log_is_syslog = true; // if false, log is a file // Set to true (when console_input_watcher is active) if console input becomes available static bool console_input_ready = false; -#ifdef SUPPORT_CGROUPS +#if SUPPORT_CGROUPS // Path of the root cgroup according to dinit. This will be dinit's own cgroup path. std::string cgroups_path; bool have_cgroups_path = false; @@ -353,7 +353,7 @@ static int process_commandline_arg(char **argv, int argc, int &i, options &opts) if (!arg_to_loglevel("--log-level", wanted_level)) return 1; log_level[DLOG_MAIN] = wanted_level; } - #ifdef SUPPORT_CGROUPS + #if SUPPORT_CGROUPS else if (strcmp(argv[i], "--cgroup-path") == 0 || strcmp(argv[i], "-b") == 0) { if (++i < argc && argv[i][0] != '\0') { cgroups_path = argv[i]; @@ -395,7 +395,7 @@ static int process_commandline_arg(char **argv, int argc, int &i, options &opts) " path to control socket\n" " --ready-fd , -F \n" " file descriptor to report readiness\n" - #ifdef SUPPORT_CGROUPS + #if SUPPORT_CGROUPS " --cgroup-path , -b \n" " cgroup base path (for resolving relative paths)\n" #endif @@ -1087,7 +1087,7 @@ static void flush_log() noexcept } } -#ifdef SUPPORT_CGROUPS +#if SUPPORT_CGROUPS static void find_cgroup_path() noexcept { @@ -1187,7 +1187,7 @@ static void printVersion() { std::cout << "Dinit version " << DINIT_VERSION << '.' << std::endl; const unsigned feature_count = 0 -#ifdef SUPPORT_CGROUPS +#if SUPPORT_CGROUPS +1 #endif #ifdef USE_UTMPX @@ -1199,7 +1199,7 @@ static void printVersion() ; if (feature_count != 0) { std::cout << "Supported features:" -#ifdef SUPPORT_CGROUPS +#if SUPPORT_CGROUPS " cgroups" #endif #ifdef USE_UTMPX diff --git a/src/run-child-proc.cc b/src/run-child-proc.cc index be443624..240d82b4 100644 --- a/src/run-child-proc.cc +++ b/src/run-child-proc.cc @@ -15,7 +15,7 @@ #include "proc-service.h" #include "mconfig.h" -#ifdef SUPPORT_CGROUPS +#if SUPPORT_CGROUPS extern std::string cgroups_path; extern bool have_cgroups_path; #endif From 696128f3484a2d2ed4c9c52028ad023fd83f198e Mon Sep 17 00:00:00 2001 From: Davin McCall Date: Sat, 12 Apr 2025 15:53:59 +1000 Subject: [PATCH 44/51] Fix #if vs #ifdef for USE_UTMPX --- src/dinit.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/dinit.cc b/src/dinit.cc index 52f667b0..4ab0232e 100644 --- a/src/dinit.cc +++ b/src/dinit.cc @@ -1190,7 +1190,7 @@ static void printVersion() #if SUPPORT_CGROUPS +1 #endif -#ifdef USE_UTMPX +#if USE_UTMPX +1 #endif #if USE_INITGROUPS @@ -1202,7 +1202,7 @@ static void printVersion() #if SUPPORT_CGROUPS " cgroups" #endif -#ifdef USE_UTMPX +#if USE_UTMPX " utmp" #endif #if USE_INITGROUPS From ddfd87f40566b2a0219b9573d832c116edeb6644 Mon Sep 17 00:00:00 2001 From: Davin McCall Date: Mon, 14 Apr 2025 22:17:08 +1000 Subject: [PATCH 45/51] shutdown: support differently named shutdown type constants NetBSD/FreeBSD use RB_HALT rather than Linux's RB_HALT_SYSTEM; For RB_POWER_OFF, FreeBSD uses RB_POWEROFF and NetBSD has RB_POWERDOWN. Resolves #441 --- src/shutdown.cc | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/shutdown.cc b/src/shutdown.cc index 6a6ab0ce..198812bb 100644 --- a/src/shutdown.cc +++ b/src/shutdown.cc @@ -415,10 +415,17 @@ void do_system_shutdown(shutdown_type_t shutdown_type) int reboot_type = RB_AUTOBOOT; // reboot #if defined(RB_POWER_OFF) if (shutdown_type == shutdown_type_t::POWEROFF) reboot_type = RB_POWER_OFF; +#elif defined(RB_POWEROFF) + // FreeBSD spells it slightly differently + if (shutdown_type == shutdown_type_t::POWEROFF) reboot_type = RB_POWEROFF; +#elif defined(RB_POWERDOWN) + // NetBSD (at least) uses RB_POWERDOWN rather than RB_POWER_OFF + if (shutdown_type == shutdown_type_t::POWEROFF) reboot_type = RB_POWERDOWN; #endif #if defined(RB_HALT_SYSTEM) if (shutdown_type == shutdown_type_t::HALT) reboot_type = RB_HALT_SYSTEM; #elif defined(RB_HALT) + // NetBSD, FreeBSD if (shutdown_type == shutdown_type_t::HALT) reboot_type = RB_HALT; #endif From c7303c415ee2daad5cb87a19c39c943e2995e3c9 Mon Sep 17 00:00:00 2001 From: Mobin Aydinfar Date: Thu, 17 Apr 2025 09:05:31 +0330 Subject: [PATCH 46/51] CI: Use ubuntu-24.04 for regular ci Ubuntu-20.04 is getting removed from GitHub CI and we need to move to newer version. See https://github.com/actions/runner-images/issues/11101 for more details. --- .github/workflows/regular_ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/regular_ci.yml b/.github/workflows/regular_ci.yml index 059efab8..5fea30b5 100644 --- a/.github/workflows/regular_ci.yml +++ b/.github/workflows/regular_ci.yml @@ -33,7 +33,7 @@ jobs: Debian-bullseye_build: - runs-on: ubuntu-20.04 + runs-on: ubuntu-24.04 container: image: debian:bullseye strategy: @@ -109,7 +109,7 @@ jobs: Alpine-latest_build: - runs-on: ubuntu-20.04 + runs-on: ubuntu-24.04 container: image: alpine:latest strategy: From e1b39ef9fe87798764ec2e4af091d0cdcf8b89b3 Mon Sep 17 00:00:00 2001 From: Davin McCall Date: Wed, 16 Apr 2025 22:05:40 +1000 Subject: [PATCH 47/51] Minor formatting/spelling --- src/dinitctl.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/dinitctl.cc b/src/dinitctl.cc index 9c098bc6..71880392 100644 --- a/src/dinitctl.cc +++ b/src/dinitctl.cc @@ -101,7 +101,7 @@ int dinitctl_main(int argc, char **argv) bool cmdline_error = false; bool show_help = argc < 2; // show help if no arguments std::string control_socket_str; - const char * control_socket_path = nullptr; + const char *control_socket_path = nullptr; bool verbose = true; bool user_dinit = (getuid() != 0); // communicate with user daemon service_dir_opt service_dir_opts; @@ -673,12 +673,12 @@ int main(int argc, char **argv) return 1; } -static const char * describe_state(bool stopped) +static const char *describe_state(bool stopped) { return stopped ? "stopped" : "started"; } -static const char * describe_verb(bool stop) +static const char *describe_verb(bool stop) { return stop ? "stop" : "start"; } @@ -1862,10 +1862,10 @@ static std::string get_service_description_dir(int socknum, cpbuffer_t &rbuffer, return result_str; } -static std::string get_service_descr_filename(int socknum, cpbuffer_t &rbuffer, handle_t serivce_handle, +static std::string get_service_descr_filename(int socknum, cpbuffer_t &rbuffer, handle_t service_handle, const char *service_name) { - std::string r = get_service_description_dir(socknum, rbuffer, serivce_handle); + std::string r = get_service_description_dir(socknum, rbuffer, service_handle); if (r.empty()) throw dinit_protocol_error(); if (r.back() != '/') From c127222d3104bdf32bcd0a83e4a4c5b9e7e64d0d Mon Sep 17 00:00:00 2001 From: davmac Date: Wed, 23 Apr 2025 12:30:21 +1000 Subject: [PATCH 48/51] parse_gid_param: switch arg order of service/parameter name Other parsing functions in load-service generally have the service name as an argument with the parameter name as the next argument. For some reason this was switched around with parse_gid_param. This already caused one bug, which is also fixed with this commit ("logfile-gid" parameter had it around the wrong way, which would have caused funky error messages in case of an invalid setting). --- src/includes/load-service.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/includes/load-service.h b/src/includes/load-service.h index 333e336c..97bfcecb 100644 --- a/src/includes/load-service.h +++ b/src/includes/load-service.h @@ -665,7 +665,7 @@ inline uid_t parse_uid_param(file_pos_ref input_pos, const std::string ¶m, } inline gid_t parse_gid_param(file_pos_ref input_pos, const std::string ¶m, - const char *setting_name, const std::string &service_name) + const std::string &service_name, const char *setting_name) { const char * gid_err_msg = "specified group id contains invalid numeric characters or is " "outside allowed range."; @@ -1540,7 +1540,7 @@ void process_service_line(settings_wrapper &settings, const char *name, const ch case setting_id_t::SOCKET_GID: { string sock_gid_s = read_setting_value(input_pos, i, end, nullptr); - settings.socket_gid = parse_gid_param(input_pos, sock_gid_s, "socket-gid", name); + settings.socket_gid = parse_gid_param(input_pos, sock_gid_s, name, "socket-gid"); break; } case setting_id_t::STOP_COMMAND: From 4a3458d6f6e44fb0ee3a9eeedf44587f393edebf Mon Sep 17 00:00:00 2001 From: Davin McCall Date: Thu, 24 Apr 2025 20:02:27 +1000 Subject: [PATCH 49/51] BUILD: update compiler requirements GCC 4.9 is actually too old and fails to build. --- BUILD | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/BUILD b/BUILD index 61f51717..d7140fca 100644 --- a/BUILD +++ b/BUILD @@ -2,7 +2,7 @@ Building Dinit =-=-=-=-=-=-=- Building Dinit should be a straight-forward process. It requires GNU make and a C++11 compiler -(GCC version 4.9 and later, or Clang ~5.0 and later, should be fine), as well as a handful of +(GCC version 11 and later, or Clang ~7.0 and later, should be fine), as well as a handful of utilities that should be available on any POSIX-compliant system; in particular, the "m4" processor is required for the manual pages. From 3af499bd023fc57b98a076d96afab13a64045e65 Mon Sep 17 00:00:00 2001 From: Davin McCall Date: Thu, 8 May 2025 08:34:12 +1000 Subject: [PATCH 50/51] Import Dasynq 2.1.2 The latest version of Dasynq has better NetBSD support and workarounds for some spurious compiler warnings. --- dasynq/include/dasynq/config.h | 2 +- dasynq/include/dasynq/interrupt.h | 14 ++++++++++---- dasynq/include/dasynq/svec.h | 10 +++++++--- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/dasynq/include/dasynq/config.h b/dasynq/include/dasynq/config.h index 257be360..6169de0d 100644 --- a/dasynq/include/dasynq/config.h +++ b/dasynq/include/dasynq/config.h @@ -53,7 +53,7 @@ // --------------------------------------------------------------------------------------------------------- #if !defined(DASYNQ_HAVE_KQUEUE) -#if defined(__OpenBSD__) || defined(__APPLE__) || defined(__FreeBSD__) +#if defined(__OpenBSD__) || defined(__APPLE__) || defined(__FreeBSD__) || defined(__NetBSD__) #define DASYNQ_HAVE_KQUEUE 1 #endif #endif diff --git a/dasynq/include/dasynq/interrupt.h b/dasynq/include/dasynq/interrupt.h index f4b83dbc..1eda9920 100644 --- a/dasynq/include/dasynq/interrupt.h +++ b/dasynq/include/dasynq/interrupt.h @@ -4,7 +4,10 @@ #include #include -#ifdef DASYNQ_HAVE_EVENTFD +#include +#include + +#if DASYNQ_HAVE_EVENTFD #include #endif @@ -24,7 +27,7 @@ template class interrup template class interrupt_channel : public Base { public: - void interrupt_wait() + void interrupt_wait() noexcept { } @@ -33,7 +36,7 @@ template class interrupt_channel : public Base template class interrupt_channel : public Base { #if !DASYNQ_HAVE_EVENTFD - static inline int create_pipe(int filedes[2]) + static inline int create_pipe(int filedes[2]) noexcept { return pipe2(filedes, O_CLOEXEC | O_NONBLOCK); } @@ -63,6 +66,9 @@ template class interrupt_channel : public Base pipe_w_fd = pipedes[1]; #else pipe_r_fd = eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK); + if (pipe_r_fd == -1) { + throw std::system_error(errno, std::system_category()); + } #endif try { @@ -106,7 +112,7 @@ template class interrupt_channel : public Base } } - void interrupt_wait() + void interrupt_wait() noexcept { #if !DASYNQ_HAVE_EVENTFD char buf[1] = { 0 }; diff --git a/dasynq/include/dasynq/svec.h b/dasynq/include/dasynq/svec.h index bfcfd4bc..c525e3de 100644 --- a/dasynq/include/dasynq/svec.h +++ b/dasynq/include/dasynq/svec.h @@ -93,9 +93,14 @@ class svector bool change_capacity(size_type c) noexcept(std::is_nothrow_move_constructible::value || std::is_nothrow_copy_constructible::value) { + _Pragma ("GCC diagnostic push") + _Pragma ("GCC diagnostic ignored \"-Walloc-size-larger-than=\"") + T *new_storage = (T *)(new (std::nothrow) char[c * sizeof(T)]); if (new_storage == nullptr) return false; + _Pragma ("GCC diagnostic pop") + // To transfer, we prefer move unless it is throwing and copy exists svec_helper::move_helper::move(array, new_storage, size_v); @@ -223,9 +228,8 @@ class svector return std::numeric_limits::max() / sizeof(T); // if we were to support allocators: - //size_t max = std::allocator_traits>::max_size(std::allocator()); - //return max / sizeof(T); - // (but not / sizeof(T) for C++17 apparently) + //size_t max = std::allocator_traits>::max_size(std::allocator()); + //return max; } void reserve(size_t amount) From 029f54d007386c815aee6b6d458d691c3bf95e15 Mon Sep 17 00:00:00 2001 From: Davin McCall Date: Thu, 8 May 2025 19:29:10 +1000 Subject: [PATCH 51/51] Version 0.19.4 --- NEWS | 23 ++++++++++++++++++++--- README.md | 2 +- build/version.conf | 4 ++-- 3 files changed, 23 insertions(+), 6 deletions(-) diff --git a/NEWS b/NEWS index c7b53a32..3f0170c6 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,23 @@ -== Version ??? +== Version 0.19.4 (beta release #8) -TBA +This release contains minor bug fixes compared to the previous release. It also adds explicit +support for NetBSD as a platform. + +Thanks to current sponsors: Paweł Zmarzły (pzmarzly), Wesley Moore (wezm), +M. Herdiansyah (konimex), Coleman McFarland (dontlaugh), q66, saolof, and private sponsors. + +New features: + * Added explicit support for NetBSD as a platform at build time, and support shutdown on NetBSD + (Emile 'iMil' Heitor). (Note that shutdown is still not built by default except on Linux). + +Fixes: + * fix for a glitch where dinitctl status would incorrectly show "no error" as the reason for + service start failure. + * fix for corrupted error message for bad values for certain service settings. + * fix building Cgroups support even when SUPPORT_CGROUPS is set to 0 (MahnoKropotkinvich). + +As usual, a number of minor fixes and improvements (in addition to those already mentioned) were +contributed by Mobin Aydinfar. == Version 0.19.3 (beta release #7) @@ -12,7 +29,7 @@ small number of documentation fixes and improvements also. Thanks to current sponsors: Paweł Zmarzły (pzmarzly), Wesley Moore (wezm), M. Herdiansyah (konimex), Coleman McFarland (dontlaugh), q66, saolof, and private sponsors. -== Version 0.19.2 +== Version 0.19.2 (beta release #6) This beta release contains minor bugfixes compared to the prior release. It does not contain various feature changes that be released in version 0.20.0 in due course. diff --git a/README.md b/README.md index db275426..3e409a6d 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # Dinit -v0.19.4pre +v0.19.4 --- ![Dinit logo](doc/dinit-logo.png) diff --git a/build/version.conf b/build/version.conf index 98bd1a9c..c2fe015e 100644 --- a/build/version.conf +++ b/build/version.conf @@ -1,4 +1,4 @@ # Included from Makefiles. -VERSION=0.19.4pre -MONTH=January +VERSION=0.19.4 +MONTH=May YEAR=2025