Skip to content

dinit-check: fix null dereference on "consumer-of: non-existent" and an igr-test for it #545

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mobin-2008 merged 2 commits into from
May 14, 2026
Merged

dinit-check: fix null dereference on "consumer-of: non-existent" and an igr-test for it #545

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2de7261
dinit-check: fix null dereference on non-existing "consumer-of" service
mobin-2008 May 8, 2026
5bad4cd
igr-tests: add the new "check-consumer-of-non-existent" test
mobin-2008 May 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/dinit-check.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ int main(int argc, char **argv)
}
else {
std::cerr << "Warning: Service '" << svc_name_record.first << "' specified as consumer of service '"
<< consumer_of_svc->name << "' which was not found.\n";
<< svc_name_record.second->consumer_of_name << "' which was not found.\n";
}
}

Expand Down
7 changes: 7 additions & 0 deletions src/igr-tests/check-consumer-of-non-existent/expected.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
Checking service: boot...
Checking service: non-existent...
Unable to load service 'non-existent': service description not found.
Performing secondary checks...
Warning: Service 'boot' specified as consumer of service 'non-existent' which was not found.
Secondary checks complete.
One or more errors/warnings issued.
4 changes: 4 additions & 0 deletions src/igr-tests/check-consumer-of-non-existent/sd/boot
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
type = process
command = /bin/sh

consumer-of: non-existent
31 changes: 22 additions & 9 deletions src/igr-tests/igr-runner.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ void check_basic_test();
void check_cycle_test();
void check_cycle2_test();
void check_lint_test();
void check_consumer_of_non_existent_test();
void reload1_test();
void reload2_test();
void no_command_error_test();
Expand All @@ -52,15 +53,17 @@ int main(int argc, char **argv)
{ "chain-to", chain_to_test }, { "force-stop", force_stop_test },
{ "restart", restart_test }, { "check-basic", check_basic_test },
{ "check-cycle", check_cycle_test }, { "check-cycle2", check_cycle2_test },
{ "check-lint", check_lint_test }, { "reload1", reload1_test },
{ "reload2", reload2_test }, { "no-command-error", no_command_error_test },
{ "add-rm-dep", add_rm_dep_test }, { "var-subst", var_subst_test },
{ "svc-start-fail", svc_start_fail_test, }, { "dep-not-found", dep_not_found_test },
{ "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 },
{ "svc-arg", svc_arg_test }, { "svc-arg-consumer", svc_arg_consumer_test },
{ "check-lint", check_lint_test },
{ "check-consumer-of-non-existent", check_consumer_of_non_existent_test },
{ "reload1", reload1_test }, { "reload2", reload2_test },
{ "no-command-error", no_command_error_test }, { "add-rm-dep", add_rm_dep_test },
{ "var-subst", var_subst_test }, { "svc-start-fail", svc_start_fail_test, },
{ "dep-not-found", dep_not_found_test }, { "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 }, { "svc-arg", svc_arg_test },
{ "svc-arg-consumer", svc_arg_consumer_test },
Comment on lines +56 to +66
Copy link
Copy Markdown
Owner

@davmac314 davmac314 May 11, 2026

Choose a reason for hiding this comment

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

Rather than change all these lines, just add the new test at the end.

Copy link
Copy Markdown
Collaborator Author

@mobin-2008 mobin-2008 May 13, 2026

Choose a reason for hiding this comment

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

Do I need to put the new test at the end in other places like function declaration and actual test function? or just for this list?

Copy link
Copy Markdown
Owner

@davmac314 davmac314 May 14, 2026

Choose a reason for hiding this comment

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

If the list here matches the order in the source code / declarations then: yes, if you move it here, you should move it there as well.

However, looking at this again I guess you've put it where you have because it keeps the dinit-check -based tests together? In that case, this is ok to leave as-is. But the commit comment could use improvement as I mentioned.

Copy link
Copy Markdown
Collaborator Author

@mobin-2008 mobin-2008 May 14, 2026

Choose a reason for hiding this comment

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

If the list here matches the order in the source code / declarations then: yes, if you move it here, you should move it there as well.

However, looking at this again I guess you've put it where you have because it keeps the dinit-check -based tests together? In that case, this is ok to leave as-is. But the commit comment could use improvement as I mentioned.

Yes, Exactly.

You mean for the first commit? yes, I improved it and I will push the changes after all things get resolved (i.e. when there is nothing to talk about and next round of review process can begin).

Copy link
Copy Markdown
Owner

@davmac314 davmac314 May 14, 2026

Choose a reason for hiding this comment

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

It is ok, only the commit comment is my concern.

Copy link
Copy Markdown
Collaborator Author

@mobin-2008 mobin-2008 May 14, 2026

Choose a reason for hiding this comment

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

So I'm going to push the changes.

{ "svc-arg-enable", svc_arg_enable_test } };
constexpr int num_tests = sizeof(tests) / sizeof(tests[0]);

Expand Down Expand Up @@ -443,6 +446,16 @@ void check_lint_test()
igr_assert(check_result.second == 1, "dinitcheck exit status == 1");
}

void check_consumer_of_non_existent_test()
{
igr_test_setup setup("check-consumer-of-non-existent");

auto check_result = run_dinitcheck("check-consumer-of-non-existent", {"-d", "sd"});
igr_assert_eq(read_file_contents(igr_input_basedir +
"/check-consumer-of-non-existent/expected.txt"), check_result.first);
igr_assert(check_result.second == 1, "dinitcheck exit status == 1");
}

void reload1_test()
{
igr_test_setup setup("reload1");
Expand Down
Loading