Skip to content

Commit b644bc3

Browse files
committed
Refactor: controller: Best practices in do_te_control()
This one is slightly more complex than other recent ones. * Use pcmk__is_set() instead of bitwise-and. * Drop some elses after early returns. * Drop a redundant "action & A_TE_START" check before checking cur_state. We would have returned already if that flag were not set. * Wait to register the TE UUID until after checking error conditions; register it only if our TE start is a success. * Drop the init_ok variable and use early returns instead. Signed-off-by: Reid Wahl <[email protected]>
1 parent ed2d910 commit b644bc3

File tree

1 file changed

+31
-38
lines changed

1 file changed

+31
-38
lines changed

daemons/controld/controld_transition.c

Lines changed: 31 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -17,23 +17,21 @@
1717
static pcmk__graph_t *
1818
create_blank_graph(void)
1919
{
20-
pcmk__graph_t *a_graph = pcmk__unpack_graph(NULL, NULL);
20+
pcmk__graph_t *graph = pcmk__unpack_graph(NULL, NULL);
2121

22-
a_graph->complete = true;
23-
a_graph->abort_reason = "DC Takeover";
24-
a_graph->completion_action = pcmk__graph_restart;
25-
return a_graph;
22+
graph->complete = true;
23+
graph->abort_reason = "DC Takeover";
24+
graph->completion_action = pcmk__graph_restart;
25+
return graph;
2626
}
2727

28-
/* A_TE_START, A_TE_STOP, O_TE_RESTART */
28+
// A_TE_START, A_TE_STOP, O_TE_RESTART
2929
void
30-
do_te_control(long long action,
31-
enum crmd_fsa_cause cause,
32-
enum crmd_fsa_state cur_state,
33-
enum crmd_fsa_input current_input, fsa_data_t * msg_data)
30+
do_te_control(long long action, enum crmd_fsa_cause cause,
31+
enum crmd_fsa_state cur_state, enum crmd_fsa_input current_input,
32+
fsa_data_t *msg_data)
3433
{
3534
cib_t *cib_conn = controld_globals.cib_conn;
36-
gboolean init_ok = TRUE;
3735

3836
if (pcmk__is_set(action, A_TE_STOP)) {
3937
pcmk__free_graph(controld_globals.transition_graph);
@@ -49,44 +47,39 @@ do_te_control(long long action,
4947
crm_info("Transitioner is now inactive");
5048
}
5149

52-
if ((action & A_TE_START) == 0) {
50+
if (!pcmk__is_set(action, A_TE_START)) {
5351
return;
54-
55-
} else if (pcmk__is_set(controld_globals.fsa_input_register,
56-
R_TE_CONNECTED)) {
52+
}
53+
if (pcmk__is_set(controld_globals.fsa_input_register, R_TE_CONNECTED)) {
5754
crm_debug("The transitioner is already active");
5855
return;
59-
60-
} else if ((action & A_TE_START) && cur_state == S_STOPPING) {
61-
crm_info("Ignoring request to start the transitioner while shutting down");
62-
return;
6356
}
64-
65-
if (controld_globals.te_uuid == NULL) {
66-
controld_globals.te_uuid = pcmk__generate_uuid();
67-
crm_info("Registering TE UUID: %s", controld_globals.te_uuid);
57+
if (cur_state == S_STOPPING) {
58+
crm_info("Ignoring request to start the transitioner while shutting "
59+
"down");
60+
return;
6861
}
69-
7062
if (cib_conn == NULL) {
7163
crm_err("Could not set CIB callbacks");
72-
init_ok = FALSE;
73-
74-
} else if (cib_conn->cmds->add_notify_callback(cib_conn,
75-
PCMK__VALUE_CIB_DIFF_NOTIFY,
76-
te_update_diff) != pcmk_ok) {
64+
return;
65+
}
66+
if (cib_conn->cmds->add_notify_callback(cib_conn,
67+
PCMK__VALUE_CIB_DIFF_NOTIFY,
68+
te_update_diff) != pcmk_ok) {
7769
crm_err("Could not set CIB notification callback");
78-
init_ok = FALSE;
70+
return;
7971
}
8072

81-
if (init_ok) {
82-
controld_register_graph_functions();
83-
pcmk__free_graph(controld_globals.transition_graph);
84-
85-
/* create a blank one */
86-
crm_debug("Transitioner is now active");
87-
controld_globals.transition_graph = create_blank_graph();
88-
controld_set_fsa_input_flags(R_TE_CONNECTED);
73+
if (controld_globals.te_uuid == NULL) {
74+
controld_globals.te_uuid = pcmk__generate_uuid();
75+
crm_info("Registering TE UUID: %s", controld_globals.te_uuid);
8976
}
77+
78+
controld_register_graph_functions();
79+
pcmk__free_graph(controld_globals.transition_graph);
80+
controld_globals.transition_graph = create_blank_graph();
81+
controld_set_fsa_input_flags(R_TE_CONNECTED);
82+
crm_debug("Transitioner is now active");
9083
}
9184

9285
/* A_TE_INVOKE, A_TE_CANCEL */

0 commit comments

Comments
 (0)