From 7056ae08bfa5cafeec9c454cb40aefa7553af6df Mon Sep 17 00:00:00 2001
From: Chris Lumens <clumens@redhat.com>
Date: Thu, 16 Jul 2020 12:53:24 -0400
Subject: [PATCH 1/4] Fix: libcrmcommon: Set out->priv to NULL in free_priv.
init won't do anything if priv is not NULL, so when the private data is
freed, also set it to NULL. This prevents segfaults when reset is
called.
---
lib/common/output_html.c | 1 +
lib/common/output_log.c | 1 +
lib/common/output_text.c | 1 +
lib/common/output_xml.c | 1 +
tools/crm_mon_curses.c | 1 +
5 files changed, 5 insertions(+)
diff --git a/lib/common/output_html.c b/lib/common/output_html.c
index c8f0088..fc06641 100644
--- a/lib/common/output_html.c
+++ b/lib/common/output_html.c
@@ -72,6 +72,7 @@ html_free_priv(pcmk__output_t *out) {
g_queue_free(priv->parent_q);
g_slist_free(priv->errors);
free(priv);
+ out->priv = NULL;
}
static bool
diff --git a/lib/common/output_log.c b/lib/common/output_log.c
index 5b45ce4..0208046 100644
--- a/lib/common/output_log.c
+++ b/lib/common/output_log.c
@@ -44,6 +44,7 @@ log_free_priv(pcmk__output_t *out) {
g_queue_free(priv->prefixes);
free(priv);
+ out->priv = NULL;
}
static bool
diff --git a/lib/common/output_text.c b/lib/common/output_text.c
index 54c409a..8f15849 100644
--- a/lib/common/output_text.c
+++ b/lib/common/output_text.c
@@ -43,6 +43,7 @@ text_free_priv(pcmk__output_t *out) {
g_queue_free(priv->parent_q);
free(priv);
+ out->priv = NULL;
}
static bool
diff --git a/lib/common/output_xml.c b/lib/common/output_xml.c
index 8565bfe..858da3f 100644
--- a/lib/common/output_xml.c
+++ b/lib/common/output_xml.c
@@ -54,6 +54,7 @@ xml_free_priv(pcmk__output_t *out) {
g_queue_free(priv->parent_q);
g_slist_free(priv->errors);
free(priv);
+ out->priv = NULL;
}
static bool
diff --git a/tools/crm_mon_curses.c b/tools/crm_mon_curses.c
index d93b847..e9cc023 100644
--- a/tools/crm_mon_curses.c
+++ b/tools/crm_mon_curses.c
@@ -46,6 +46,7 @@ curses_free_priv(pcmk__output_t *out) {
g_queue_free(priv->parent_q);
free(priv);
+ out->priv = NULL;
}
static bool
--
1.8.3.1
From 3779152993ca0e88dc407c918882568217f1b630 Mon Sep 17 00:00:00 2001
From: Chris Lumens <clumens@redhat.com>
Date: Thu, 16 Jul 2020 13:50:24 -0400
Subject: [PATCH 2/4] Fix: libcrmcommon: Make reset and finish work more
similarly.
When finish is called for HTML and XML output formats, various extra
nodes and headers are added, errors are added, etc. None of this stuff
happens on reset. For the HTML format, this also means things like the
CGI headers and title don't get added when reset is called. Make these
two functions much more similar.
Regression in 2.0.3.
See: rhbz#1857728
---
lib/common/output_html.c | 26 ++++++++++++++++----------
lib/common/output_xml.c | 30 ++++++++++++++++--------------
2 files changed, 32 insertions(+), 24 deletions(-)
diff --git a/lib/common/output_html.c b/lib/common/output_html.c
index fc06641..6127df2 100644
--- a/lib/common/output_html.c
+++ b/lib/common/output_html.c
@@ -113,18 +113,11 @@ add_error_node(gpointer data, gpointer user_data) {
}
static void
-html_finish(pcmk__output_t *out, crm_exit_t exit_status, bool print, void **copy_dest) {
+finish_reset_common(pcmk__output_t *out, crm_exit_t exit_status, bool print) {
private_data_t *priv = out->priv;
htmlNodePtr head_node = NULL;
htmlNodePtr charset_node = NULL;
- /* If root is NULL, html_init failed and we are being called from pcmk__output_free
- * in the pcmk__output_new path.
- */
- if (priv == NULL || priv->root == NULL) {
- return;
- }
-
if (cgi_output && print) {
fprintf(out->dest, "Content-Type: text/html\n\n");
}
@@ -174,6 +167,20 @@ html_finish(pcmk__output_t *out, crm_exit_t exit_status, bool print, void **copy
if (print) {
htmlDocDump(out->dest, priv->root->doc);
}
+}
+
+static void
+html_finish(pcmk__output_t *out, crm_exit_t exit_status, bool print, void **copy_dest) {
+ private_data_t *priv = out->priv;
+
+ /* If root is NULL, html_init failed and we are being called from pcmk__output_free
+ * in the pcmk__output_new path.
+ */
+ if (priv == NULL || priv->root == NULL) {
+ return;
+ }
+
+ finish_reset_common(out, exit_status, print);
if (copy_dest != NULL) {
*copy_dest = copy_xml(priv->root);
@@ -185,8 +192,7 @@ html_reset(pcmk__output_t *out) {
CRM_ASSERT(out != NULL);
if (out->priv != NULL) {
- private_data_t *priv = out->priv;
- htmlDocDump(out->dest, priv->root->doc);
+ finish_reset_common(out, CRM_EX_OK, true);
}
html_free_priv(out);
diff --git a/lib/common/output_xml.c b/lib/common/output_xml.c
index 858da3f..b64a71d 100644
--- a/lib/common/output_xml.c
+++ b/lib/common/output_xml.c
@@ -106,17 +106,10 @@ add_error_node(gpointer data, gpointer user_data) {
}
static void
-xml_finish(pcmk__output_t *out, crm_exit_t exit_status, bool print, void **copy_dest) {
+finish_reset_common(pcmk__output_t *out, crm_exit_t exit_status, bool print) {
xmlNodePtr node;
private_data_t *priv = out->priv;
- /* If root is NULL, xml_init failed and we are being called from pcmk__output_free
- * in the pcmk__output_new path.
- */
- if (priv == NULL || priv->root == NULL) {
- return;
- }
-
if (legacy_xml) {
GSList *node = priv->errors;
@@ -148,6 +141,20 @@ xml_finish(pcmk__output_t *out, crm_exit_t exit_status, bool print, void **copy_
fprintf(out->dest, "%s", buf);
free(buf);
}
+}
+
+static void
+xml_finish(pcmk__output_t *out, crm_exit_t exit_status, bool print, void **copy_dest) {
+ private_data_t *priv = out->priv;
+
+ /* If root is NULL, xml_init failed and we are being called from pcmk__output_free
+ * in the pcmk__output_new path.
+ */
+ if (priv == NULL || priv->root == NULL) {
+ return;
+ }
+
+ finish_reset_common(out, exit_status, print);
if (copy_dest != NULL) {
*copy_dest = copy_xml(priv->root);
@@ -156,15 +163,10 @@ xml_finish(pcmk__output_t *out, crm_exit_t exit_status, bool print, void **copy_
static void
xml_reset(pcmk__output_t *out) {
- char *buf = NULL;
-
CRM_ASSERT(out != NULL);
if (out->priv != NULL) {
- private_data_t *priv = out->priv;
- buf = dump_xml_formatted_with_text(priv->root);
- fprintf(out->dest, "%s", buf);
- free(buf);
+ finish_reset_common(out, CRM_EX_OK, true);
}
xml_free_priv(out);
--
1.8.3.1
From 0f8e4ca5d9a429c934f1e91a1bdf572efd07e0db Mon Sep 17 00:00:00 2001
From: Chris Lumens <clumens@redhat.com>
Date: Thu, 16 Jul 2020 16:09:08 -0400
Subject: [PATCH 3/4] Fix: tools, libcrmcommon: Reopen the output dest on
reset.
This is needed when running crm_mon as a daemon. When we do a reset,
we need to clear out any existing output destination and start writing
again from the beginning. This really only matters when the destination
is a file.
The extra freopen at the end of crm_mon is to handle when crm_mon is
killed. We need to reset the output file to its beginning before
calling finish.
---
lib/common/output_html.c | 3 +++
lib/common/output_log.c | 3 +++
lib/common/output_text.c | 3 +++
lib/common/output_xml.c | 3 +++
tools/crm_mon.c | 9 +++++++++
5 files changed, 21 insertions(+)
diff --git a/lib/common/output_html.c b/lib/common/output_html.c
index 6127df2..6e21031 100644
--- a/lib/common/output_html.c
+++ b/lib/common/output_html.c
@@ -191,6 +191,9 @@ static void
html_reset(pcmk__output_t *out) {
CRM_ASSERT(out != NULL);
+ out->dest = freopen(NULL, "w", out->dest);
+ CRM_ASSERT(out->dest != NULL);
+
if (out->priv != NULL) {
finish_reset_common(out, CRM_EX_OK, true);
}
diff --git a/lib/common/output_log.c b/lib/common/output_log.c
index 0208046..8422ac2 100644
--- a/lib/common/output_log.c
+++ b/lib/common/output_log.c
@@ -72,6 +72,9 @@ static void
log_reset(pcmk__output_t *out) {
CRM_ASSERT(out != NULL);
+ out->dest = freopen(NULL, "w", out->dest);
+ CRM_ASSERT(out->dest != NULL);
+
log_free_priv(out);
log_init(out);
}
diff --git a/lib/common/output_text.c b/lib/common/output_text.c
index 8f15849..2f7e5b0 100644
--- a/lib/common/output_text.c
+++ b/lib/common/output_text.c
@@ -75,6 +75,9 @@ static void
text_reset(pcmk__output_t *out) {
CRM_ASSERT(out != NULL);
+ out->dest = freopen(NULL, "w", out->dest);
+ CRM_ASSERT(out->dest != NULL);
+
text_free_priv(out);
text_init(out);
}
diff --git a/lib/common/output_xml.c b/lib/common/output_xml.c
index b64a71d..9f8e01b 100644
--- a/lib/common/output_xml.c
+++ b/lib/common/output_xml.c
@@ -165,6 +165,9 @@ static void
xml_reset(pcmk__output_t *out) {
CRM_ASSERT(out != NULL);
+ out->dest = freopen(NULL, "w", out->dest);
+ CRM_ASSERT(out->dest != NULL);
+
if (out->priv != NULL) {
finish_reset_common(out, CRM_EX_OK, true);
}
diff --git a/tools/crm_mon.c b/tools/crm_mon.c
index b2e143b..10624c1 100644
--- a/tools/crm_mon.c
+++ b/tools/crm_mon.c
@@ -2014,6 +2014,10 @@ mon_refresh_display(gpointer user_data)
break;
}
+ if (options.daemonize) {
+ out->reset(out);
+ }
+
stonith_history_free(stonith_history);
stonith_history = NULL;
pe_reset_working_set(mon_data_set);
@@ -2179,6 +2183,11 @@ clean_up(crm_exit_t exit_code)
* crm_mon to be able to do so.
*/
if (out != NULL) {
+ if (options.daemonize) {
+ out->dest = freopen(NULL, "w", out->dest);
+ CRM_ASSERT(out->dest != NULL);
+ }
+
switch (output_format) {
case mon_output_cgi:
case mon_output_html:
--
1.8.3.1
From b655c039414d2c7af77c3532222b04684ef1f3d0 Mon Sep 17 00:00:00 2001
From: Chris Lumens <clumens@redhat.com>
Date: Fri, 17 Jul 2020 10:58:32 -0400
Subject: [PATCH 4/4] Fix: tools: Add the http-equiv header to crm_mon at the
right time.
This header is only getting added on termination, which is not a lot of
help if you're running crm_mon in daemonize mode. Putting this header
in at the right time requires a couple changes:
* pcmk__html_add_header doesn't need a parent argument. It was not
being used in the first place.
* The extra_headers list in output_html.c should not be freed in the
reset function. This means it would get freed after every time the
daemonized output is refreshed, which means the header would have to be
added every time too. extra_headers will now only be freed when the
program exits. This is a behavior change, but I can't see why it's
going to be a problem.
* To support that, we need to copy each item in the extra_headers list
when it gets added to the output XML document. This prevents segfaults
when we later free that document.
* handle_html_output no longer needs to exist. That function only
existed to add the http-equiv header at the end, which is wrong.
---
include/crm/common/output.h | 5 ++---
lib/common/output_html.c | 7 ++++---
tools/crm_mon.c | 26 +++++++-------------------
3 files changed, 13 insertions(+), 25 deletions(-)
diff --git a/include/crm/common/output.h b/include/crm/common/output.h
index e7c9417..186bcfe 100644
--- a/include/crm/common/output.h
+++ b/include/crm/common/output.h
@@ -703,15 +703,14 @@ pcmk__output_create_html_node(pcmk__output_t *out, const char *element_name, con
* the following code would generate the tag "<meta http-equiv='refresh' content='19'>":
*
* \code
- * pcmk__html_add_header(parent, "meta", "http-equiv", "refresh", "content", "19", NULL);
+ * pcmk__html_add_header("meta", "http-equiv", "refresh", "content", "19", NULL);
* \endcode
*
- * \param[in,out] parent The node that will be the parent of the new node.
* \param[in] name The HTML tag for the new node.
* \param[in] ... A NULL-terminated key/value list of attributes.
*/
void
-pcmk__html_add_header(xmlNodePtr parent, const char *name, ...)
+pcmk__html_add_header(const char *name, ...)
G_GNUC_NULL_TERMINATED;
#ifdef __cplusplus
diff --git a/lib/common/output_html.c b/lib/common/output_html.c
index 6e21031..259e412 100644
--- a/lib/common/output_html.c
+++ b/lib/common/output_html.c
@@ -139,7 +139,7 @@ finish_reset_common(pcmk__output_t *out, crm_exit_t exit_status, bool print) {
/* Add any extra header nodes the caller might have created. */
for (int i = 0; i < g_slist_length(extra_headers); i++) {
- xmlAddChild(head_node, g_slist_nth_data(extra_headers, i));
+ xmlAddChild(head_node, xmlCopyNode(g_slist_nth_data(extra_headers, i), 1));
}
/* Stylesheets are included two different ways. The first is via a built-in
@@ -185,6 +185,8 @@ html_finish(pcmk__output_t *out, crm_exit_t exit_status, bool print, void **copy
if (copy_dest != NULL) {
*copy_dest = copy_xml(priv->root);
}
+
+ g_slist_free_full(extra_headers, (GDestroyNotify) xmlFreeNode);
}
static void
@@ -199,7 +201,6 @@ html_reset(pcmk__output_t *out) {
}
html_free_priv(out);
- g_slist_free_full(extra_headers, (GDestroyNotify) xmlFreeNode);
html_init(out);
}
@@ -412,7 +413,7 @@ pcmk__output_create_html_node(pcmk__output_t *out, const char *element_name, con
}
void
-pcmk__html_add_header(xmlNodePtr parent, const char *name, ...) {
+pcmk__html_add_header(const char *name, ...) {
htmlNodePtr header_node;
va_list ap;
diff --git a/tools/crm_mon.c b/tools/crm_mon.c
index 10624c1..7fd2b9c 100644
--- a/tools/crm_mon.c
+++ b/tools/crm_mon.c
@@ -1346,6 +1346,12 @@ main(int argc, char **argv)
options.mon_ops |= mon_op_print_timing | mon_op_inactive_resources;
}
+ if ((output_format == mon_output_html || output_format == mon_output_cgi) &&
+ out->dest != stdout) {
+ pcmk__html_add_header("meta", "http-equiv", "refresh", "content",
+ crm_itoa(options.reconnect_msec/1000), NULL);
+ }
+
crm_info("Starting %s", crm_system_name);
if (cib) {
@@ -2106,15 +2112,6 @@ clean_up_connections(void)
}
}
-static void
-handle_html_output(crm_exit_t exit_code) {
- xmlNodePtr html = NULL;
-
- pcmk__html_add_header(html, "meta", "http-equiv", "refresh", "content",
- crm_itoa(options.reconnect_msec/1000), NULL);
- out->finish(out, exit_code, true, (void **) &html);
-}
-
/*
* De-init ncurses, disconnect from the CIB manager, disconnect fencing,
* deallocate memory and show usage-message if requested.
@@ -2188,16 +2185,7 @@ clean_up(crm_exit_t exit_code)
CRM_ASSERT(out->dest != NULL);
}
- switch (output_format) {
- case mon_output_cgi:
- case mon_output_html:
- handle_html_output(exit_code);
- break;
-
- default:
- out->finish(out, exit_code, true, NULL);
- break;
- }
+ out->finish(out, exit_code, true, NULL);
pcmk__output_free(out);
pcmk__unregister_formats();
--
1.8.3.1