Skip to content

Commit c915e77

Browse files
nrwahl2nrwahl2
committed
Refactor: libcrmcommon: pcmk__output_create_xml_node() 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 8ad5cd3 commit c915e77

12 files changed

Lines changed: 447 additions & 532 deletions

File tree

‎include/crm/common/output_internal.h‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -779,11 +779,9 @@ pcmk__output_xml_add_node_copy(pcmk__output_t *out, xmlNodePtr node);
779779
*
780780
* \param[in,out] out The output functions structure.
781781
* \param[in] name The name of the node to be created.
782-
* \param[in] ... Name/value pairs to set as XML properties.
783782
*/
784-
xmlNodePtr
785-
pcmk__output_create_xml_node(pcmk__output_t *out, const char *name, ...)
786-
G_GNUC_NULL_TERMINATED;
783+
xmlNode *
784+
pcmk__output_create_xml_node(pcmk__output_t *out, const char *name);
787785

788786
/*!
789787
* \internal

‎lib/common/options_display.c‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -298,9 +298,9 @@ add_possible_values_xml(pcmk__output_t *out,
298298
values = g_strsplit(option->values, ", ", 0);
299299

300300
for (gchar **value = values; *value != NULL; value++) {
301-
pcmk__output_create_xml_node(out, PCMK_XE_OPTION,
302-
PCMK_XA_VALUE, *value,
303-
NULL);
301+
xmlNode *xml = pcmk__output_create_xml_node(out, PCMK_XE_OPTION);
302+
303+
pcmk__xe_set(xml, PCMK_XA_VALUE, *value);
304304
}
305305

306306
g_strfreev(values);
@@ -437,9 +437,8 @@ add_option_metadata_xml(pcmk__output_t *out,
437437
pcmk__output_xml_create_parent(out, PCMK_XE_DEPRECATED);
438438

439439
if (replaced_with != NULL) {
440-
pcmk__output_create_xml_node(out, PCMK_XE_REPLACED_WITH,
441-
PCMK_XA_NAME, replaced_with,
442-
NULL);
440+
xml = pcmk__output_create_xml_node(out, PCMK_XE_REPLACED_WITH);
441+
pcmk__xe_set(xml, PCMK_XA_NAME, replaced_with);
443442
}
444443
pcmk__output_xml_pop_parent(out);
445444
}

‎lib/common/output_html.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -407,7 +407,7 @@ html_is_quiet(pcmk__output_t *out) {
407407
static void
408408
html_spacer(pcmk__output_t *out) {
409409
pcmk__assert(out != NULL);
410-
pcmk__output_create_xml_node(out, "br", NULL);
410+
pcmk__output_create_xml_node(out, "br");
411411
}
412412

413413
static void

‎lib/common/output_xml.c‎

Lines changed: 16 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -264,17 +264,17 @@ xml_subprocess_output(pcmk__output_t *out, int exit_status,
264264
static void
265265
xml_version(pcmk__output_t *out)
266266
{
267-
const char *author = "Andrew Beekhof and the Pacemaker project "
268-
"contributors";
267+
xmlNode *xml = NULL;
268+
269269
pcmk__assert(out != NULL);
270270

271-
pcmk__output_create_xml_node(out, PCMK_XE_VERSION,
272-
PCMK_XA_PROGRAM, "Pacemaker",
273-
PCMK_XA_VERSION, PACEMAKER_VERSION,
274-
PCMK_XA_AUTHOR, author,
275-
PCMK_XA_BUILD, BUILD_VERSION,
276-
PCMK_XA_FEATURES, CRM_FEATURES,
277-
NULL);
271+
xml = pcmk__output_create_xml_node(out, PCMK_XE_VERSION);
272+
pcmk__xe_set(xml, PCMK_XA_PROGRAM, "Pacemaker");
273+
pcmk__xe_set(xml, PCMK_XA_VERSION, PACEMAKER_VERSION);
274+
pcmk__xe_set(xml, PCMK_XA_AUTHOR,
275+
"Andrew Beekhof and the Pacemaker project contributors");
276+
pcmk__xe_set(xml, PCMK_XA_BUILD, BUILD_VERSION);
277+
pcmk__xe_set(xml, PCMK_XA_FEATURES, CRM_FEATURES);
278278
}
279279

280280
G_GNUC_PRINTF(2, 3)
@@ -311,7 +311,7 @@ xml_output_xml(pcmk__output_t *out, const char *name, const char *buf) {
311311

312312
pcmk__assert(out != NULL);
313313

314-
parent = pcmk__output_create_xml_node(out, name, NULL);
314+
parent = pcmk__output_create_xml_node(out, name);
315315
if (parent == NULL) {
316316
return;
317317
}
@@ -475,7 +475,7 @@ pcmk__output_xml_create_parent(pcmk__output_t *out, const char *name)
475475
pcmk__assert(out != NULL);
476476
CRM_CHECK(pcmk__str_any_of(out->fmt_name, "xml", "html", NULL), return NULL);
477477

478-
node = pcmk__output_create_xml_node(out, name, NULL);
478+
node = pcmk__output_create_xml_node(out, name);
479479

480480
pcmk__output_xml_push_parent(out, node);
481481
return node;
@@ -500,11 +500,10 @@ pcmk__output_xml_add_node_copy(pcmk__output_t *out, xmlNodePtr node) {
500500
pcmk__xml_copy(parent, node);
501501
}
502502

503-
xmlNodePtr
504-
pcmk__output_create_xml_node(pcmk__output_t *out, const char *name, ...) {
505-
xmlNodePtr node = NULL;
503+
xmlNode *
504+
pcmk__output_create_xml_node(pcmk__output_t *out, const char *name)
505+
{
506506
private_data_t *priv = NULL;
507-
va_list args;
508507

509508
pcmk__assert((out != NULL) && (out->priv != NULL));
510509
CRM_CHECK(pcmk__str_any_of(out->fmt_name, "xml", "html", NULL), return NULL);
@@ -513,12 +512,7 @@ pcmk__output_create_xml_node(pcmk__output_t *out, const char *name, ...) {
513512

514513
priv = out->priv;
515514

516-
node = pcmk__xe_create(g_queue_peek_tail(priv->parent_q), name);
517-
va_start(args, name);
518-
pcmk__xe_set_propv(node, args);
519-
va_end(args);
520-
521-
return node;
515+
return pcmk__xe_create(g_queue_peek_tail(priv->parent_q), name);
522516
}
523517

524518
xmlNodePtr
@@ -528,7 +522,7 @@ pcmk__output_create_xml_text_node(pcmk__output_t *out, const char *name, const c
528522
pcmk__assert(out != NULL);
529523
CRM_CHECK(pcmk__str_any_of(out->fmt_name, "xml", "html", NULL), return NULL);
530524

531-
node = pcmk__output_create_xml_node(out, name, NULL);
525+
node = pcmk__output_create_xml_node(out, name);
532526
pcmk__xe_set_content(node, "%s", content);
533527
return node;
534528
}

‎lib/fencing/st_output.c‎

Lines changed: 24 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -311,13 +311,9 @@ full_history_xml(pcmk__output_t *out, va_list args)
311311

312312
PCMK__OUTPUT_LIST_FOOTER(out, rc);
313313
} else {
314-
char *rc_s = pcmk__itoa(history_rc);
315-
316-
pcmk__output_create_xml_node(out, PCMK_XE_FENCE_HISTORY,
317-
PCMK_XA_STATUS, rc_s,
318-
NULL);
319-
free(rc_s);
314+
xmlNode *xml = pcmk__output_create_xml_node(out, PCMK_XE_FENCE_HISTORY);
320315

316+
pcmk__xe_set_int(xml, PCMK_XA_STATUS, history_rc);
321317
rc = pcmk_rc_ok;
322318
}
323319

@@ -364,11 +360,10 @@ last_fenced_xml(pcmk__output_t *out, va_list args) {
364360

365361
if (when) {
366362
char *buf = timespec_string(when, 0, false);
363+
xmlNode *xml = pcmk__output_create_xml_node(out, PCMK_XE_LAST_FENCED);
367364

368-
pcmk__output_create_xml_node(out, PCMK_XE_LAST_FENCED,
369-
PCMK_XA_TARGET, target,
370-
PCMK_XA_WHEN, buf,
371-
NULL);
365+
pcmk__xe_set(xml, PCMK_XA_TARGET, target);
366+
pcmk__xe_set(xml, PCMK_XA_WHEN, buf);
372367

373368
free(buf);
374369
return pcmk_rc_ok;
@@ -478,45 +473,39 @@ stonith_event_xml(pcmk__output_t *out, va_list args)
478473
const char *succeeded G_GNUC_UNUSED = va_arg(args, const char *);
479474
uint32_t show_opts G_GNUC_UNUSED = va_arg(args, uint32_t);
480475

481-
xmlNodePtr node = NULL;
476+
xmlNode *xml = NULL;
482477

483-
node = pcmk__output_create_xml_node(out, PCMK_XE_FENCE_EVENT,
484-
PCMK_XA_ACTION, event->action,
485-
PCMK_XA_TARGET, event->target,
486-
PCMK_XA_CLIENT, event->client,
487-
PCMK_XA_ORIGIN, event->origin,
488-
NULL);
478+
xml = pcmk__output_create_xml_node(out, PCMK_XE_FENCE_EVENT);
479+
pcmk__xe_set(xml, PCMK_XA_ACTION, event->action);
480+
pcmk__xe_set(xml, PCMK_XA_TARGET, event->target);
481+
pcmk__xe_set(xml, PCMK_XA_CLIENT, event->client);
482+
pcmk__xe_set(xml, PCMK_XA_ORIGIN, event->origin);
489483

490484
switch (event->state) {
491485
case st_failed:
492-
pcmk__xe_set(node, PCMK_XA_STATUS, PCMK_VALUE_FAILED);
493-
pcmk__xe_set(node, PCMK_XA_EXIT_REASON, event->exit_reason);
486+
pcmk__xe_set(xml, PCMK_XA_STATUS, PCMK_VALUE_FAILED);
487+
pcmk__xe_set(xml, PCMK_XA_EXIT_REASON, event->exit_reason);
494488
break;
495489

496490
case st_done:
497-
pcmk__xe_set(node, PCMK_XA_STATUS, PCMK_VALUE_SUCCESS);
491+
pcmk__xe_set(xml, PCMK_XA_STATUS, PCMK_VALUE_SUCCESS);
498492
break;
499493

500-
default: {
501-
char *state = pcmk__itoa(event->state);
502-
503-
pcmk__xe_set(node, PCMK_XA_STATUS, PCMK_VALUE_PENDING);
504-
pcmk__xe_set(node, PCMK_XA_EXTENDED_STATUS, state);
505-
506-
free(state);
494+
default:
495+
pcmk__xe_set(xml, PCMK_XA_STATUS, PCMK_VALUE_PENDING);
496+
pcmk__xe_set_int(xml, PCMK_XA_EXTENDED_STATUS, event->state);
507497
break;
508-
}
509498
}
510499

511500
if (event->delegate != NULL) {
512-
pcmk__xe_set(node, PCMK_XA_DELEGATE, event->delegate);
501+
pcmk__xe_set(xml, PCMK_XA_DELEGATE, event->delegate);
513502
}
514503

515504
if ((event->state == st_failed) || (event->state == st_done)) {
516505
char *time_s = timespec_string(event->completed, event->completed_nsec,
517506
true);
518507

519-
pcmk__xe_set(node, PCMK_XA_COMPLETED, time_s);
508+
pcmk__xe_set(xml, PCMK_XA_COMPLETED, time_s);
520509
free(time_s);
521510
}
522511

@@ -583,17 +572,13 @@ validate_agent_xml(pcmk__output_t *out, va_list args) {
583572
const char *error_output = va_arg(args, const char *);
584573
int rc = va_arg(args, int);
585574

586-
const char *valid = pcmk__btoa(rc == pcmk_ok);
587-
xmlNodePtr node = pcmk__output_create_xml_node(out, PCMK_XE_VALIDATE,
588-
PCMK_XA_AGENT, agent,
589-
PCMK_XA_VALID, valid,
590-
NULL);
575+
xmlNode *xml = pcmk__output_create_xml_node(out, PCMK_XE_VALIDATE);
591576

592-
if (device != NULL) {
593-
pcmk__xe_set(node, PCMK_XA_DEVICE, device);
594-
}
577+
pcmk__xe_set(xml, PCMK_XA_AGENT, agent);
578+
pcmk__xe_set_bool(xml, PCMK_XA_VALID, (rc == pcmk_ok));
579+
pcmk__xe_set(xml, PCMK_XA_DEVICE, device);
595580

596-
pcmk__output_xml_push_parent(out, node);
581+
pcmk__output_xml_push_parent(out, xml);
597582
out->subprocess_output(out, rc, output, error_output);
598583
pcmk__output_xml_pop_parent(out);
599584

0 commit comments

Comments
 (0)