Skip to content

Commit 730fa7c

Browse files
poetteringpoettering
committed
machined: some modernizations
A couple of minor modernizations: 1. Don't unnecessarily export functions we don't call outside of machined.c 2. Use cleanup logic for the manager object. 3. Propagate errors properly from manager_new(). So far if sd_event_new() returns EMFILE/ENFILE for some reason we would have logged that as log_oom(), which isn#t right, really. 4. Handle SIGTERM/SIGINT cleanly. It's easy, and prettier then letting the kernel just abort us. It also makes it possible to valgrind machined properly.
1 parent edac2c4 commit 730fa7c

File tree

3 files changed

+46
-48
lines changed

3 files changed

+46
-48
lines changed

‎src/machine/machinectl.c‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2638,12 +2638,14 @@ static int set_limit(int argc, char *argv[], void *userdata) {
26382638
uint64_t limit;
26392639
int r;
26402640

2641+
polkit_agent_open_if_enabled(arg_transport, arg_ask_password);
2642+
26412643
if (STR_IN_SET(argv[argc-1], "-", "none", "infinity"))
26422644
limit = (uint64_t) -1;
26432645
else {
26442646
r = parse_size(argv[argc-1], 1024, &limit);
26452647
if (r < 0)
2646-
return log_error("Failed to parse size: %s", argv[argc-1]);
2648+
return log_error_errno(r, "Failed to parse size: %s", argv[argc-1]);
26472649
}
26482650

26492651
if (argc > 2)
@@ -2670,10 +2672,8 @@ static int set_limit(int argc, char *argv[], void *userdata) {
26702672
NULL,
26712673
"t", limit);
26722674

2673-
if (r < 0) {
2674-
log_error("Could not set limit: %s", bus_error_message(&error, -r));
2675-
return r;
2676-
}
2675+
if (r < 0)
2676+
return log_error_errno(r, "Could not set limit: %s", bus_error_message(&error, r));
26772677

26782678
return 0;
26792679
}
@@ -2688,6 +2688,8 @@ static int clean_images(int argc, char *argv[], void *userdata) {
26882688
unsigned c = 0;
26892689
int r;
26902690

2691+
polkit_agent_open_if_enabled(arg_transport, arg_ask_password);
2692+
26912693
r = sd_bus_message_new_method_call(
26922694
bus,
26932695
&m,
@@ -3139,7 +3141,7 @@ int main(int argc, char*argv[]) {
31393141
goto finish;
31403142
}
31413143

3142-
sd_bus_set_allow_interactive_authorization(bus, arg_ask_password);
3144+
(void) sd_bus_set_allow_interactive_authorization(bus, arg_ask_password);
31433145

31443146
r = machinectl_main(argc, argv, bus);
31453147

‎src/machine/machined.c‎

Lines changed: 38 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -26,35 +26,45 @@
2626
#include "signal-util.h"
2727
#include "special.h"
2828

29-
Manager *manager_new(void) {
30-
Manager *m;
29+
static Manager* manager_unref(Manager *m);
30+
DEFINE_TRIVIAL_CLEANUP_FUNC(Manager*, manager_unref);
31+
32+
static int manager_new(Manager **ret) {
33+
_cleanup_(manager_unrefp) Manager *m = NULL;
3134
int r;
3235

36+
assert(ret);
37+
3338
m = new0(Manager, 1);
3439
if (!m)
35-
return NULL;
40+
return -ENOMEM;
3641

3742
m->machines = hashmap_new(&string_hash_ops);
3843
m->machine_units = hashmap_new(&string_hash_ops);
3944
m->machine_leaders = hashmap_new(NULL);
4045

41-
if (!m->machines || !m->machine_units || !m->machine_leaders) {
42-
manager_free(m);
43-
return NULL;
44-
}
46+
if (!m->machines || !m->machine_units || !m->machine_leaders)
47+
return -ENOMEM;
4548

4649
r = sd_event_default(&m->event);
47-
if (r < 0) {
48-
manager_free(m);
49-
return NULL;
50-
}
50+
if (r < 0)
51+
return r;
52+
53+
r = sd_event_add_signal(m->event, NULL, SIGINT, NULL, NULL);
54+
if (r < 0)
55+
return r;
56+
57+
r = sd_event_add_signal(m->event, NULL, SIGTERM, NULL, NULL);
58+
if (r < 0)
59+
return r;
5160

52-
sd_event_set_watchdog(m->event, true);
61+
(void) sd_event_set_watchdog(m->event, true);
5362

54-
return m;
63+
*ret = TAKE_PTR(m);
64+
return 0;
5565
}
5666

57-
void manager_free(Manager *m) {
67+
static Manager* manager_unref(Manager *m) {
5868
Machine *machine;
5969

6070
assert(m);
@@ -80,7 +90,7 @@ void manager_free(Manager *m) {
8090
sd_bus_unref(m->bus);
8191
sd_event_unref(m->event);
8292

83-
free(m);
93+
return mfree(m);
8494
}
8595

8696
static int manager_add_host_machine(Manager *m) {
@@ -121,7 +131,7 @@ static int manager_add_host_machine(Manager *m) {
121131
return 0;
122132
}
123133

124-
int manager_enumerate_machines(Manager *m) {
134+
static int manager_enumerate_machines(Manager *m) {
125135
_cleanup_closedir_ DIR *d = NULL;
126136
struct dirent *de;
127137
int r = 0;
@@ -268,7 +278,7 @@ static int manager_connect_bus(Manager *m) {
268278
return 0;
269279
}
270280

271-
void manager_gc(Manager *m, bool drop_not_started) {
281+
static void manager_gc(Manager *m, bool drop_not_started) {
272282
Machine *machine;
273283

274284
assert(m);
@@ -292,7 +302,7 @@ void manager_gc(Manager *m, bool drop_not_started) {
292302
}
293303
}
294304

295-
int manager_startup(Manager *m) {
305+
static int manager_startup(Manager *m) {
296306
Machine *machine;
297307
Iterator i;
298308
int r;
@@ -328,7 +338,7 @@ static bool check_idle(void *userdata) {
328338
return hashmap_isempty(m->machines);
329339
}
330340

331-
int manager_run(Manager *m) {
341+
static int manager_run(Manager *m) {
332342
assert(m);
333343

334344
return bus_event_loop_with_idle(
@@ -340,7 +350,7 @@ int manager_run(Manager *m) {
340350
}
341351

342352
int main(int argc, char *argv[]) {
343-
Manager *m = NULL;
353+
_cleanup_(manager_unrefp) Manager *m = NULL;
344354
int r;
345355

346356
log_set_target(LOG_TARGET_AUTO);
@@ -356,18 +366,16 @@ int main(int argc, char *argv[]) {
356366
goto finish;
357367
}
358368

359-
/* Always create the directories people can create inotify
360-
* watches in. Note that some applications might check for the
361-
* existence of /run/systemd/machines/ to determine whether
362-
* machined is available, so please always make sure this
363-
* check stays in. */
364-
mkdir_label("/run/systemd/machines", 0755);
369+
/* Always create the directories people can create inotify watches in. Note that some applications might check
370+
* for the existence of /run/systemd/machines/ to determine whether machined is available, so please always
371+
* make sure this check stays in. */
372+
(void) mkdir_label("/run/systemd/machines", 0755);
365373

366-
assert_se(sigprocmask_many(SIG_BLOCK, NULL, SIGCHLD, -1) >= 0);
374+
assert_se(sigprocmask_many(SIG_BLOCK, NULL, SIGCHLD, SIGTERM, SIGINT, -1) >= 0);
367375

368-
m = manager_new();
369-
if (!m) {
370-
r = log_oom();
376+
r = manager_new(&m);
377+
if (r < 0) {
378+
log_error_errno(r, "Failed to allocate manager object: %m");
371379
goto finish;
372380
}
373381

@@ -388,7 +396,5 @@ int main(int argc, char *argv[]) {
388396
log_debug("systemd-machined stopped as pid "PID_FMT, getpid_cached());
389397

390398
finish:
391-
manager_free(m);
392-
393399
return r < 0 ? EXIT_FAILURE : EXIT_SUCCESS;
394400
}

‎src/machine/machined.h‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -43,17 +43,7 @@ struct Manager {
4343
unsigned n_operations;
4444
};
4545

46-
Manager *manager_new(void);
47-
void manager_free(Manager *m);
48-
4946
int manager_add_machine(Manager *m, const char *name, Machine **_machine);
50-
int manager_enumerate_machines(Manager *m);
51-
52-
int manager_startup(Manager *m);
53-
int manager_run(Manager *m);
54-
55-
void manager_gc(Manager *m, bool drop_not_started);
56-
5747
int manager_get_machine_by_pid(Manager *m, pid_t pid, Machine **machine);
5848

5949
extern const sd_bus_vtable manager_vtable[];

0 commit comments

Comments
 (0)