Skip to content

Commit 8ad5cd3

Browse files
nrwahl2nrwahl2
committed
Refactor: libcrmcommon: pcmk__output_xml_create_parent() drops list arg
I don't think the variadic argument helps readability much. It avoids the need to store the result in an xmlNode * variable in the caller, which can save a line. But it requires an extra line for the NULL terminator, and it requires indenting each line much farther, which is one reason we have so many temp variables. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
1 parent a871bb3 commit 8ad5cd3

15 files changed

Lines changed: 203 additions & 238 deletions

File tree

‎daemons/pacemakerd/pacemakerd.c‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -67,12 +67,12 @@ PCMK__OUTPUT_ARGS("features")
6767
static int
6868
pacemakerd_features_xml(pcmk__output_t *out, va_list args) {
6969
gchar **feature_list = g_strsplit(CRM_FEATURES, " ", 0);
70+
xmlNode *xml = pcmk__output_xml_create_parent(out, PCMK_XE_PACEMAKERD);
71+
72+
pcmk__xe_set(xml, PCMK_XA_VERSION, PACEMAKER_VERSION);
73+
pcmk__xe_set(xml, PCMK_XA_BUILD, BUILD_VERSION);
74+
pcmk__xe_set(xml, PCMK_XA_FEATURE_SET, CRM_FEATURE_SET);
7075

71-
pcmk__output_xml_create_parent(out, PCMK_XE_PACEMAKERD,
72-
PCMK_XA_VERSION, PACEMAKER_VERSION,
73-
PCMK_XA_BUILD, BUILD_VERSION,
74-
PCMK_XA_FEATURE_SET, CRM_FEATURE_SET,
75-
NULL);
7676
out->begin_list(out, NULL, NULL, PCMK_XE_FEATURES);
7777

7878
for (char **s = feature_list; *s != NULL; s++) {

‎include/crm/common/output_internal.h‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -757,11 +757,9 @@ void pcmk__output_set_log_filter(pcmk__output_t *out, const char *file,
757757
*
758758
* \param[in,out] out The output functions structure.
759759
* \param[in] name The name of the node to be created.
760-
* \param[in] ... Name/value pairs to set as XML properties.
761760
*/
762-
xmlNodePtr
763-
pcmk__output_xml_create_parent(pcmk__output_t *out, const char *name, ...)
764-
G_GNUC_NULL_TERMINATED;
761+
xmlNode *
762+
pcmk__output_xml_create_parent(pcmk__output_t *out, const char *name);
765763

766764
/*!
767765
* \internal

‎lib/common/options_display.c‎

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,7 @@ add_option_metadata_xml(pcmk__output_t *out,
348348
const pcmk__cluster_option_t *option,
349349
const char *replaced_with)
350350
{
351+
xmlNode *xml = NULL;
351352
const char *type = option->type;
352353
const char *desc_long = option->description_long;
353354
const char *desc_short = option->description_short;
@@ -427,14 +428,13 @@ add_option_metadata_xml(pcmk__output_t *out,
427428
generated_s = NULL;
428429
}
429430

430-
pcmk__output_xml_create_parent(out, PCMK_XE_PARAMETER,
431-
PCMK_XA_NAME, option->name,
432-
PCMK_XA_ADVANCED, advanced_s,
433-
PCMK_XA_GENERATED, generated_s,
434-
NULL);
431+
xml = pcmk__output_xml_create_parent(out, PCMK_XE_PARAMETER);
432+
pcmk__xe_set(xml, PCMK_XA_NAME, option->name);
433+
pcmk__xe_set(xml, PCMK_XA_ADVANCED, advanced_s);
434+
pcmk__xe_set(xml, PCMK_XA_GENERATED, generated_s);
435435

436436
if (deprecated && !legacy) {
437-
pcmk__output_xml_create_parent(out, PCMK_XE_DEPRECATED, NULL);
437+
pcmk__output_xml_create_parent(out, PCMK_XE_DEPRECATED);
438438

439439
if (replaced_with != NULL) {
440440
pcmk__output_create_xml_node(out, PCMK_XE_REPLACED_WITH,
@@ -446,10 +446,9 @@ add_option_metadata_xml(pcmk__output_t *out,
446446
add_desc_xml(out, true, desc_long);
447447
add_desc_xml(out, false, desc_short);
448448

449-
pcmk__output_xml_create_parent(out, PCMK_XE_CONTENT,
450-
PCMK_XA_TYPE, type,
451-
PCMK_XA_DEFAULT, option->default_value,
452-
NULL);
449+
xml = pcmk__output_xml_create_parent(out, PCMK_XE_CONTENT);
450+
pcmk__xe_set(xml, PCMK_XA_TYPE, type);
451+
pcmk__xe_set(xml, PCMK_XA_DEFAULT, option->default_value);
453452

454453
add_possible_values_xml(out, option);
455454

@@ -485,6 +484,7 @@ PCMK__OUTPUT_ARGS("option-list", "const char *", "const char *", "const char *",
485484
static int
486485
option_list_xml(pcmk__output_t *out, va_list args)
487486
{
487+
xmlNode *xml = NULL;
488488
const char *name = va_arg(args, const char *);
489489
const char *desc_short = va_arg(args, const char *);
490490
const char *desc_long = va_arg(args, const char *);
@@ -495,16 +495,15 @@ option_list_xml(pcmk__output_t *out, va_list args)
495495
pcmk__assert((out != NULL) && (name != NULL) && (desc_short != NULL)
496496
&& (desc_long != NULL) && (option_list != NULL));
497497

498-
pcmk__output_xml_create_parent(out, PCMK_XE_RESOURCE_AGENT,
499-
PCMK_XA_NAME, name,
500-
PCMK_XA_VERSION, PACEMAKER_VERSION,
501-
NULL);
498+
xml = pcmk__output_xml_create_parent(out, PCMK_XE_RESOURCE_AGENT);
499+
pcmk__xe_set(xml, PCMK_XA_NAME, name);
500+
pcmk__xe_set(xml, PCMK_XA_VERSION, PACEMAKER_VERSION);
502501

503502
pcmk__output_create_xml_text_node(out, PCMK_XE_VERSION, PCMK_OCF_VERSION);
504503
add_desc_xml(out, true, desc_long);
505504
add_desc_xml(out, false, desc_short);
506505

507-
pcmk__output_xml_create_parent(out, PCMK_XE_PARAMETERS, NULL);
506+
pcmk__output_xml_create_parent(out, PCMK_XE_PARAMETERS);
508507

509508
for (const pcmk__cluster_option_t *option = option_list;
510509
option->name != NULL; option++) {

‎lib/common/output_html.c‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ html_init(pcmk__output_t *out) {
125125
g_queue_push_tail(priv->parent_q, priv->root);
126126
priv->errors = NULL;
127127

128-
pcmk__output_xml_create_parent(out, "body", NULL);
128+
pcmk__output_xml_create_parent(out, "body");
129129

130130
return true;
131131
}
@@ -324,7 +324,7 @@ html_begin_list(pcmk__output_t *out, const char *singular_noun,
324324
*/
325325
q_len = g_queue_get_length(priv->parent_q);
326326
if (q_len > 2) {
327-
pcmk__output_xml_create_parent(out, "li", NULL);
327+
pcmk__output_xml_create_parent(out, "li");
328328
}
329329

330330
if (format != NULL) {
@@ -346,7 +346,9 @@ html_begin_list(pcmk__output_t *out, const char *singular_noun,
346346
free(buf);
347347
}
348348

349-
node = pcmk__output_xml_create_parent(out, "ul", NULL);
349+
node = pcmk__output_xml_create_parent(out, "ul");
350+
351+
// @FIXME This looks like an incorrect double-push; check this
350352
g_queue_push_tail(priv->parent_q, node);
351353
}
352354

‎lib/common/output_xml.c‎

Lines changed: 11 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -208,18 +208,14 @@ xml_finish(pcmk__output_t *out, crm_exit_t exit_status, bool print, void **copy_
208208
node = node->next;
209209
}
210210
} else {
211-
char *rc_as_str = pcmk__itoa(exit_status);
212-
213211
node = pcmk__xe_create(priv->root, PCMK_XE_STATUS);
214-
pcmk__xe_set(node, PCMK_XA_CODE, rc_as_str);
212+
pcmk__xe_set_int(node, PCMK_XA_CODE, exit_status);
215213
pcmk__xe_set(node, PCMK_XA_MESSAGE, crm_exit_str(exit_status));
216214

217215
if (g_slist_length(priv->errors) > 0) {
218216
xmlNodePtr errors_node = pcmk__xe_create(node, PCMK_XE_ERRORS);
219217
g_slist_foreach(priv->errors, add_error_node, (gpointer) errors_node);
220218
}
221-
222-
free(rc_as_str);
223219
}
224220

225221
if (print) {
@@ -246,15 +242,11 @@ static void
246242
xml_subprocess_output(pcmk__output_t *out, int exit_status,
247243
const char *proc_stdout, const char *proc_stderr) {
248244
xmlNodePtr node, child_node;
249-
char *rc_as_str = NULL;
250245

251246
pcmk__assert(out != NULL);
252247

253-
rc_as_str = pcmk__itoa(exit_status);
254-
255-
node = pcmk__output_xml_create_parent(out, PCMK_XE_COMMAND,
256-
PCMK_XA_CODE, rc_as_str,
257-
NULL);
248+
node = pcmk__output_xml_create_parent(out, PCMK_XE_COMMAND);
249+
pcmk__xe_set_int(node, PCMK_XA_CODE, exit_status);
258250

259251
if (proc_stdout != NULL) {
260252
child_node = pcmk__xe_create(node, PCMK_XE_OUTPUT);
@@ -267,8 +259,6 @@ xml_subprocess_output(pcmk__output_t *out, int exit_status,
267259
pcmk__xe_set_content(child_node, "%s", proc_stderr);
268260
pcmk__xe_set(child_node, PCMK_XA_SOURCE, "stderr");
269261
}
270-
271-
free(rc_as_str);
272262
}
273263

274264
static void
@@ -360,11 +350,12 @@ xml_begin_list(pcmk__output_t *out, const char *singular_noun, const char *plura
360350
}
361351

362352
if (priv->list_element) {
363-
pcmk__output_xml_create_parent(out, PCMK_XE_LIST,
364-
PCMK_XA_NAME, name,
365-
NULL);
353+
xmlNode *xml = pcmk__output_xml_create_parent(out, PCMK_XE_LIST);
354+
355+
pcmk__xe_set(xml, PCMK_XA_NAME, name);
356+
366357
} else {
367-
pcmk__output_xml_create_parent(out, name, NULL);
358+
pcmk__output_xml_create_parent(out, name);
368359
}
369360

370361
g_free(name);
@@ -476,20 +467,16 @@ pcmk__mk_xml_output(char **argv) {
476467
return retval;
477468
}
478469

479-
xmlNodePtr
480-
pcmk__output_xml_create_parent(pcmk__output_t *out, const char *name, ...) {
481-
va_list args;
470+
xmlNode *
471+
pcmk__output_xml_create_parent(pcmk__output_t *out, const char *name)
472+
{
482473
xmlNodePtr node = NULL;
483474

484475
pcmk__assert(out != NULL);
485476
CRM_CHECK(pcmk__str_any_of(out->fmt_name, "xml", "html", NULL), return NULL);
486477

487478
node = pcmk__output_create_xml_node(out, name, NULL);
488479

489-
va_start(args, name);
490-
pcmk__xe_set_propv(node, args);
491-
va_end(args);
492-
493480
pcmk__output_xml_push_parent(out, node);
494481
return node;
495482
}

‎lib/lrmd/lrmd_output.c‎

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -46,11 +46,12 @@ lrmd__alternatives_list_xml(pcmk__output_t *out, va_list args) {
4646
lrmd_list_t *list = va_arg(args, lrmd_list_t *);
4747
const char *agent_spec = va_arg(args, const char *);
4848

49+
xmlNode *xml = NULL;
4950
int rc = pcmk_rc_ok;
5051

51-
pcmk__output_xml_create_parent(out, PCMK_XE_PROVIDERS,
52-
PCMK_XA_FOR, agent_spec,
53-
NULL);
52+
xml = pcmk__output_xml_create_parent(out, PCMK_XE_PROVIDERS);
53+
pcmk__xe_set(xml, PCMK_XA_FOR, agent_spec);
54+
5455
rc = xml_list(out, list, PCMK_XE_PROVIDER);
5556
pcmk__output_xml_pop_parent(out);
5657
return rc;
@@ -72,15 +73,14 @@ lrmd__agents_list_xml(pcmk__output_t *out, va_list args) {
7273
const char *agent_spec = va_arg(args, const char *);
7374
const char *provider = va_arg(args, const char *);
7475

76+
xmlNode *xml = NULL;
7577
int rc = pcmk_rc_ok;
76-
xmlNodePtr node = NULL;
7778

78-
node = pcmk__output_xml_create_parent(out, PCMK_XE_AGENTS,
79-
PCMK_XA_STANDARD, agent_spec,
80-
NULL);
79+
xml = pcmk__output_xml_create_parent(out, PCMK_XE_AGENTS);
80+
pcmk__xe_set(xml, PCMK_XA_STANDARD, agent_spec);
8181

8282
if (!pcmk__str_empty(provider)) {
83-
pcmk__xe_set(node, PCMK_XA_PROVIDER, provider);
83+
pcmk__xe_set(xml, PCMK_XA_PROVIDER, provider);
8484
}
8585

8686
rc = xml_list(out, list, PCMK_XE_AGENT);
@@ -110,14 +110,12 @@ lrmd__providers_list_xml(pcmk__output_t *out, va_list args) {
110110
lrmd_list_t *list = va_arg(args, lrmd_list_t *);
111111
const char *agent_spec = va_arg(args, const char *);
112112

113+
xmlNode *xml = NULL;
113114
int rc = pcmk_rc_ok;
114-
xmlNodePtr node = pcmk__output_xml_create_parent(out, PCMK_XE_PROVIDERS,
115-
PCMK_XA_STANDARD, "ocf",
116-
NULL);
117115

118-
if (agent_spec != NULL) {
119-
pcmk__xe_set(node, PCMK_XA_AGENT, agent_spec);
120-
}
116+
xml = pcmk__output_xml_create_parent(out, PCMK_XE_PROVIDERS);
117+
pcmk__xe_set(xml, PCMK_XA_STANDARD, "ocf");
118+
pcmk__xe_set(xml, PCMK_XA_AGENT, agent_spec);
121119

122120
rc = xml_list(out, list, PCMK_XE_PROVIDER);
123121
pcmk__output_xml_pop_parent(out);

‎lib/pacemaker/pcmk_output.c‎

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -616,7 +616,7 @@ locations_and_colocations_xml(pcmk__output_t *out, va_list args)
616616
rsc = uber_parent(rsc);
617617
}
618618

619-
pcmk__output_xml_create_parent(out, PCMK_XE_CONSTRAINTS, NULL);
619+
pcmk__output_xml_create_parent(out, PCMK_XE_CONSTRAINTS);
620620
do_locations_list_xml(out, rsc, false);
621621

622622
pe__clear_resource_flags_on_all(rsc->priv->scheduler,
@@ -1593,7 +1593,7 @@ inject_modify_config_xml(pcmk__output_t *out, va_list args)
15931593
return pcmk_rc_no_output;
15941594
}
15951595

1596-
node = pcmk__output_xml_create_parent(out, PCMK_XE_MODIFICATIONS, NULL);
1596+
node = pcmk__output_xml_create_parent(out, PCMK_XE_MODIFICATIONS);
15971597

15981598
if (quorum) {
15991599
pcmk__xe_set(node, PCMK_XA_QUORUM, quorum);
@@ -2414,6 +2414,7 @@ ticket_attribute_xml(pcmk__output_t *out, va_list args)
24142414
const char *ticket_id = va_arg(args, const char *);
24152415
const char *name = va_arg(args, const char *);
24162416
const char *value = va_arg(args, const char *);
2417+
xmlNode *xml = NULL;
24172418

24182419
/* Create:
24192420
* <tickets>
@@ -2422,9 +2423,11 @@ ticket_attribute_xml(pcmk__output_t *out, va_list args)
24222423
* </ticket>
24232424
* </tickets>
24242425
*/
2425-
pcmk__output_xml_create_parent(out, PCMK_XE_TICKETS, NULL);
2426-
pcmk__output_xml_create_parent(out, PCMK_XE_TICKET,
2427-
PCMK_XA_ID, ticket_id, NULL);
2426+
pcmk__output_xml_create_parent(out, PCMK_XE_TICKETS);
2427+
2428+
xml = pcmk__output_xml_create_parent(out, PCMK_XE_TICKET);
2429+
pcmk__xe_set(xml, PCMK_XA_ID, ticket_id);
2430+
24282431
pcmk__output_create_xml_node(out, PCMK_XA_ATTRIBUTE,
24292432
PCMK_XA_NAME, name,
24302433
PCMK_XA_VALUE, value,
@@ -2489,10 +2492,12 @@ add_ticket_element_with_constraints(xmlNode *node, void *userdata)
24892492
{
24902493
pcmk__output_t *out = (pcmk__output_t *) userdata;
24912494
const char *ticket_id = pcmk__xe_get(node, PCMK_XA_TICKET);
2495+
xmlNode *xml = NULL;
2496+
2497+
xml = pcmk__output_xml_create_parent(out, PCMK_XE_TICKET);
2498+
pcmk__xe_set(xml, PCMK_XA_ID, ticket_id);
24922499

2493-
pcmk__output_xml_create_parent(out, PCMK_XE_TICKET,
2494-
PCMK_XA_ID, ticket_id, NULL);
2495-
pcmk__output_xml_create_parent(out, PCMK_XE_CONSTRAINTS, NULL);
2500+
pcmk__output_xml_create_parent(out, PCMK_XE_CONSTRAINTS);
24962501
pcmk__output_xml_add_node_copy(out, node);
24972502

24982503
/* Pop two parents so now we are back under the <tickets> element */
@@ -2529,7 +2534,7 @@ ticket_constraints_xml(pcmk__output_t *out, va_list args)
25292534
* ...
25302535
* </tickets>
25312536
*/
2532-
pcmk__output_xml_create_parent(out, PCMK_XE_TICKETS, NULL);
2537+
pcmk__output_xml_create_parent(out, PCMK_XE_TICKETS);
25332538

25342539
if (pcmk__xe_is(constraint_xml, PCMK__XE_XPATH_QUERY)) {
25352540
/* Iterate through the list of children once to create all the
@@ -2552,7 +2557,7 @@ ticket_constraints_xml(pcmk__output_t *out, va_list args)
25522557
* ...
25532558
* </resources>
25542559
*/
2555-
pcmk__output_xml_create_parent(out, PCMK_XE_RESOURCES, NULL);
2560+
pcmk__output_xml_create_parent(out, PCMK_XE_RESOURCES);
25562561
pcmk__xe_foreach_child(constraint_xml, NULL, add_resource_element, out);
25572562
pcmk__output_xml_pop_parent(out);
25582563

@@ -2563,7 +2568,7 @@ ticket_constraints_xml(pcmk__output_t *out, va_list args)
25632568
add_ticket_element_with_constraints(constraint_xml, out);
25642569
pcmk__output_xml_pop_parent(out);
25652570

2566-
pcmk__output_xml_create_parent(out, PCMK_XE_RESOURCES, NULL);
2571+
pcmk__output_xml_create_parent(out, PCMK_XE_RESOURCES);
25672572
add_resource_element(constraint_xml, out);
25682573
pcmk__output_xml_pop_parent(out);
25692574
}
@@ -2610,7 +2615,7 @@ ticket_state_xml(pcmk__output_t *out, va_list args)
26102615
* ...
26112616
* </tickets>
26122617
*/
2613-
pcmk__output_xml_create_parent(out, PCMK_XE_TICKETS, NULL);
2618+
pcmk__output_xml_create_parent(out, PCMK_XE_TICKETS);
26142619

26152620
if (state_xml->children != NULL) {
26162621
/* Iterate through the list of children once to create all the

0 commit comments

Comments
 (0)