diff --git a/daemons/based/based_callbacks.c b/daemons/based/based_callbacks.c index 13646eb4be0..1d34055d9d3 100644 --- a/daemons/based/based_callbacks.c +++ b/daemons/based/based_callbacks.c @@ -414,7 +414,8 @@ cib_digester_cb(gpointer data) free(ping_digest); ping_digest = NULL; ping_modified_since = FALSE; - snprintf(buffer, 32, "%" PRIu64, ping_seq); + pcmk__assert(snprintf(buffer, 32, "%" PRIu64, ping_seq) >= 0); + crm_trace("Requesting peer digests (%s)", buffer); crm_xml_add(ping, PCMK__XA_T, PCMK__VALUE_CIB); diff --git a/daemons/execd/cts-exec-helper.c b/daemons/execd/cts-exec-helper.c index 47ab5bbe382..9a032871365 100644 --- a/daemons/execd/cts-exec-helper.c +++ b/daemons/execd/cts-exec-helper.c @@ -1,5 +1,5 @@ /* - * Copyright 2012-2024 the Pacemaker project contributors + * Copyright 2012-2025 the Pacemaker project contributors * * The version control history for this file may have further details. * @@ -160,8 +160,6 @@ static GOptionEntry api_call_entries[] = { static GMainLoop *mainloop = NULL; static lrmd_t *lrmd_conn = NULL; -static char event_buf_v0[1024]; - static crm_exit_t test_exit(crm_exit_t exit_code) { @@ -174,15 +172,6 @@ test_exit(crm_exit_t exit_code) printf(fmt "\n" , ##args); \ } -#define report_event(event) \ - snprintf(event_buf_v0, sizeof(event_buf_v0), "NEW_EVENT event_type:%s rsc_id:%s action:%s rc:%s op_status:%s", \ - lrmd_event_type2str(event->type), \ - event->rsc_id, \ - event->op_type ? event->op_type : "none", \ - crm_exit_str((crm_exit_t) event->rc), \ - pcmk_exec_status_str(event->op_status)); \ - crm_info("%s", event_buf_v0); - static void test_shutdown(int nsig) { @@ -193,12 +182,20 @@ test_shutdown(int nsig) static void read_events(lrmd_event_data_t * event) { - report_event(event); - if (options.listen) { - if (pcmk__str_eq(options.listen, event_buf_v0, pcmk__str_casei)) { - print_result("LISTEN EVENT SUCCESSFUL"); - test_exit(CRM_EX_OK); - } + char buf[1024] = { '\0', }; + + pcmk__assert(snprintf(buf, sizeof(buf), + "NEW_EVENT event_type:%s rsc_id:%s action:%s rc:%s " + "op_status:%s", + lrmd_event_type2str(event->type), event->rsc_id, + pcmk__s(event->op_type, "none"), + crm_exit_str((crm_exit_t) event->rc), + pcmk_exec_status_str(event->op_status)) >= 0); + crm_info("%s", buf); + + if (options.listen && pcmk__str_eq(options.listen, buf, pcmk__str_casei)) { + print_result("LISTEN EVENT SUCCESSFUL"); + test_exit(CRM_EX_OK); } if (exec_call_id && (event->call_id == exec_call_id)) { diff --git a/daemons/execd/remoted_tls.c b/daemons/execd/remoted_tls.c index eede13edaec..13c867f3e63 100644 --- a/daemons/execd/remoted_tls.c +++ b/daemons/execd/remoted_tls.c @@ -319,8 +319,8 @@ bind_and_listen(struct addrinfo *addr) static int get_address_info(const char *bind_name, int port, struct addrinfo **res) { - int rc; - char port_str[6]; // at most "65535" + int rc = pcmk_rc_ok; + char *port_s = pcmk__itoa(port); struct addrinfo hints; memset(&hints, 0, sizeof(struct addrinfo)); @@ -329,17 +329,16 @@ get_address_info(const char *bind_name, int port, struct addrinfo **res) hints.ai_socktype = SOCK_STREAM; hints.ai_protocol = IPPROTO_TCP; - snprintf(port_str, sizeof(port_str), "%d", port); - rc = getaddrinfo(bind_name, port_str, &hints, res); + rc = getaddrinfo(bind_name, port_s, &hints, res); rc = pcmk__gaierror2rc(rc); if (rc != pcmk_rc_ok) { crm_err("Unable to get IP address(es) for %s: %s", (bind_name? bind_name : "local node"), pcmk_rc_str(rc)); - return rc; } - return pcmk_rc_ok; + free(port_s); + return rc; } int diff --git a/daemons/fenced/cts-fence-helper.c b/daemons/fenced/cts-fence-helper.c index cc83fd32a31..42c1730324e 100644 --- a/daemons/fenced/cts-fence-helper.c +++ b/daemons/fenced/cts-fence-helper.c @@ -497,7 +497,8 @@ test_async_monitor(int check_event) static void test_register_async_devices(int check_event) { - char buf[16] = { 0, }; + char *off_timeout_s = pcmk__itoa(MAINLOOP_DEFAULT_TIMEOUT + + CUSTOM_TIMEOUT_ADDITION); stonith_key_value_t *params = NULL; params = stonith__key_value_add(params, PCMK_STONITH_HOST_MAP, @@ -518,12 +519,12 @@ test_register_async_devices(int check_event) "custom_timeout_node1=1,2"); params = stonith__key_value_add(params, "mode", "fail"); params = stonith__key_value_add(params, "delay", "1000"); - snprintf(buf, sizeof(buf) - 1, "%d", MAINLOOP_DEFAULT_TIMEOUT + CUSTOM_TIMEOUT_ADDITION); - params = stonith__key_value_add(params, "pcmk_off_timeout", buf); + params = stonith__key_value_add(params, "pcmk_off_timeout", off_timeout_s); st->cmds->register_device(st, st_opts, "false_custom_timeout", "stonith-ng", "fence_dummy", params); stonith__key_value_freeall(params, true, true); + free(off_timeout_s); mainloop_test_done(__func__, true); } diff --git a/daemons/fenced/fenced_commands.c b/daemons/fenced/fenced_commands.c index 68c9a6df592..9934760ea5e 100644 --- a/daemons/fenced/fenced_commands.c +++ b/daemons/fenced/fenced_commands.c @@ -272,7 +272,7 @@ get_action_timeout(const fenced_device_t *device, const char *action, int default_timeout) { if (action && device && device->params) { - char buffer[64] = { 0, }; + char *timeout_param = NULL; const char *value = NULL; /* If "reboot" was requested but the device does not support it, @@ -286,8 +286,10 @@ get_action_timeout(const fenced_device_t *device, const char *action, } /* If the device config specified an action-specific timeout, use it */ - snprintf(buffer, sizeof(buffer), "pcmk_%s_timeout", action); - value = g_hash_table_lookup(device->params, buffer); + timeout_param = crm_strdup_printf("pcmk_%s_timeout", action); + value = g_hash_table_lookup(device->params, timeout_param); + free(timeout_param); + if (value) { long long timeout_ms = crm_get_msec(value); return (int) QB_MIN(pcmk__timeout_ms2s(timeout_ms), INT_MAX); diff --git a/daemons/pacemakerd/pcmkd_corosync.c b/daemons/pacemakerd/pcmkd_corosync.c index 7e54c42c4a8..f0e68d749f4 100644 --- a/daemons/pacemakerd/pcmkd_corosync.c +++ b/daemons/pacemakerd/pcmkd_corosync.c @@ -1,5 +1,5 @@ /* - * Copyright 2010-2024 the Pacemaker project contributors + * Copyright 2010-2025 the Pacemaker project contributors * * The version control history for this file may have further details. * @@ -360,9 +360,11 @@ pacemakerd_read_config(void) " No group found for user %s", CRM_DAEMON_USER); } else { - char key[PATH_MAX]; - snprintf(key, PATH_MAX, "uidgid.gid.%u", gid); + char *key = crm_strdup_printf("uidgid.gid.%lld", (long long) gid); + rc = cmap_set_uint8(local_handle, key, 1); + free(key); + if (rc != CS_OK) { crm_warn("Could not authorize group with Corosync: %s " QB_XS " group=%u rc=%d", pcmk__cs_err_str(rc), gid, rc); diff --git a/doc/sphinx/Pacemaker_Explained/alerts.rst b/doc/sphinx/Pacemaker_Explained/alerts.rst index 27000ed9410..b573bcf2d4f 100644 --- a/doc/sphinx/Pacemaker_Explained/alerts.rst +++ b/doc/sphinx/Pacemaker_Explained/alerts.rst @@ -114,14 +114,27 @@ whether and how Pacemaker calls them. | | | recipient of that alert, that recipient will not be | | | | used. *(since 2.1.6)* | +------------------+---------------+-----------------------------------------------------+ - | timestamp-format | %H:%M:%S.%06N | .. index:: | + | timestamp-format | %H:%M:%S.%6N | .. index:: | | | | single: alert; meta-attribute, timestamp-format | | | | single: meta-attribute; timestamp-format (alert) | | | | single: timestamp-format; alert meta-attribute | | | | | | | | Format the cluster will use when sending the | | | | event's timestamp to the agent. This is a string as | - | | | used with the ``date(1)`` command. | + | | | used with the ``date(1)`` command, with the | + | | | following extension. ``"%xN"``, where ``x`` is a | + | | | number with ``1 <= x <= 6``, prints the fractional | + | | | seconds component of the timestamp at ``10^(-x)`` | + | | | resolution, without a decimal point (``'.'``). | + | | | Values are truncated toward zero, not rounded. | + | | | | + | | | Note: This is implemented using ``strftime()`` with | + | | | a 128-character buffer. If any format specifier's | + | | | expansion requires more than 128 characters, or if | + | | | any specifier expands to an empty string, then the | + | | | timestamp is discarded. (Expanding to an empty | + | | | string is not an error, but there is no way to | + | | | distinguish this from a too-small buffer.) | +------------------+---------------+-----------------------------------------------------+ | timeout | 30s | .. index:: | | | | single: alert; meta-attribute, timeout | diff --git a/include/crm/common/internal.h b/include/crm/common/internal.h index ceb831c41d2..c333787d4a2 100644 --- a/include/crm/common/internal.h +++ b/include/crm/common/internal.h @@ -74,7 +74,7 @@ char *pcmk__format_nvpair(const char *name, const char *value, pid_t pcmk__procfs_pid_of(const char *name); unsigned int pcmk__procfs_num_cores(void); -int pcmk__procfs_pid2path(pid_t pid, char path[], size_t path_size); +int pcmk__procfs_pid2path(pid_t pid, char **path); bool pcmk__procfs_has_pids(void); DIR *pcmk__procfs_fd_dir(void); void pcmk__sysrq_trigger(char t); @@ -101,7 +101,6 @@ bool pcmk__throttle_load_avg(float *load); */ int pcmk__pid_active(pid_t pid, const char *daemon); -int pcmk__read_pidfile(const char *filename, pid_t *pid); int pcmk__pidfile_matches(const char *filename, pid_t expected_pid, const char *expected_name, pid_t *pid); int pcmk__lock_pidfile(const char *filename, const char *name); diff --git a/include/crm/common/iso8601_internal.h b/include/crm/common/iso8601_internal.h index 41afc60d0e2..7226e8b1662 100644 --- a/include/crm/common/iso8601_internal.h +++ b/include/crm/common/iso8601_internal.h @@ -1,5 +1,5 @@ /* - * Copyright 2015-2024 the Pacemaker project contributors + * Copyright 2015-2025 the Pacemaker project contributors * * The version control history for this file may have further details. * @@ -22,9 +22,6 @@ extern "C" { typedef struct pcmk__time_us pcmk__time_hr_t; -pcmk__time_hr_t *pcmk__time_hr_convert(pcmk__time_hr_t *target, - const crm_time_t *dt); -void pcmk__time_set_hr_dt(crm_time_t *target, const pcmk__time_hr_t *hr_dt); pcmk__time_hr_t *pcmk__time_hr_now(time_t *epoch); pcmk__time_hr_t *pcmk__time_hr_new(const char *date_time); void pcmk__time_hr_free(pcmk__time_hr_t *hr_dt); diff --git a/lib/common/attrs.c b/lib/common/attrs.c index aa50831c8b8..e3d832d57c4 100644 --- a/lib/common/attrs.c +++ b/lib/common/attrs.c @@ -1,5 +1,5 @@ /* - * Copyright 2011-2024 the Pacemaker project contributors + * Copyright 2011-2025 the Pacemaker project contributors * * The version control history for this file may have further details. * @@ -38,18 +38,22 @@ const char * pcmk__node_attr_target(const char *name) { if (name == NULL || pcmk__strcase_any_of(name, "auto", "localhost", NULL)) { - char buf[128] = OCF_RESKEY_PREFIX; - size_t offset = sizeof(OCF_RESKEY_PREFIX) - 1; + char *buf = NULL; char *target_var = crm_meta_name(PCMK_META_CONTAINER_ATTRIBUTE_TARGET); char *phys_var = crm_meta_name(PCMK__META_PHYSICAL_HOST); const char *target = NULL; const char *host_physical = NULL; - snprintf(buf + offset, sizeof(buf) - offset, "%s", target_var); + buf = crm_strdup_printf(OCF_RESKEY_PREFIX "%s", target_var); target = getenv(buf); + free(buf); - snprintf(buf + offset, sizeof(buf) - offset, "%s", phys_var); + buf = crm_strdup_printf(OCF_RESKEY_PREFIX "%s", phys_var); host_physical = getenv(buf); + free(buf); + + free(target_var); + free(phys_var); // It is important to use the name by which the scheduler knows us if (host_physical @@ -63,8 +67,6 @@ pcmk__node_attr_target(const char *name) name = host_pcmk; } } - free(target_var); - free(phys_var); // TODO? Call pcmk__cluster_local_node_name() if name == NULL // (currently would require linkage against libcrmcluster) diff --git a/lib/common/iso8601.c b/lib/common/iso8601.c index b738c0b2851..b3ffa49060a 100644 --- a/lib/common/iso8601.c +++ b/lib/common/iso8601.c @@ -479,60 +479,58 @@ crm_time_get_isoweek(const crm_time_t *dt, uint32_t *y, uint32_t *w, * \internal * \brief Print "." to a buffer * - * \param[in] sec Seconds - * \param[in] usec Microseconds (must be of same sign as \p sec and of - * absolute value less than \p QB_TIME_US_IN_SEC) - * \param[in,out] buf Result buffer - * \param[in,out] offset Current offset within \p buf + * \param[in] sec Seconds + * \param[in] usec Microseconds (must be of same sign as \p sec and of + * absolute value less than \c QB_TIME_US_IN_SEC) + * \param[in,out] buf Result buffer */ static inline void -sec_usec_as_string(long long sec, int usec, char *buf, size_t *offset) +sec_usec_as_string(long long sec, int usec, GString *buf) { - *offset += snprintf(buf + *offset, DATE_MAX - *offset, "%s%lld.%06d", - ((sec == 0) && (usec < 0))? "-" : "", - sec, QB_ABS(usec)); + /* A negative value smaller than -1 second should have the negative sign + * before the 0, not before the usec part + */ + if ((sec == 0) && (usec < 0)) { + g_string_append_c(buf, '-'); + } + g_string_append_printf(buf, "%lld.%06d", sec, QB_ABS(usec)); } /*! * \internal * \brief Get a string representation of a duration * - * \param[in] dt Time object to interpret as a duration - * \param[in] usec Microseconds to add to \p dt - * \param[in] show_usec Whether to include microseconds in \p result - * \param[out] result Where to store the result string + * \param[in] dt Time object to interpret as a duration + * \param[in] usec Microseconds to add to \p dt + * \param[in] show_usec Whether to include microseconds in \p buf + * \param[in,out] buf Result buffer */ static void -crm_duration_as_string(const crm_time_t *dt, int usec, bool show_usec, - char *result) +duration_as_string(const crm_time_t *dt, int usec, bool show_usec, GString *buf) { - size_t offset = 0; - pcmk__assert(valid_sec_usec(dt->seconds, usec)); if (dt->years) { - offset += snprintf(result + offset, DATE_MAX - offset, "%4d year%s ", - dt->years, pcmk__plural_s(dt->years)); + g_string_append_printf(buf, "%4d year%s ", + dt->years, pcmk__plural_s(dt->years)); } if (dt->months) { - offset += snprintf(result + offset, DATE_MAX - offset, "%2d month%s ", - dt->months, pcmk__plural_s(dt->months)); + g_string_append_printf(buf, "%2d month%s ", + dt->months, pcmk__plural_s(dt->months)); } if (dt->days) { - offset += snprintf(result + offset, DATE_MAX - offset, "%2d day%s ", - dt->days, pcmk__plural_s(dt->days)); + g_string_append_printf(buf, "%2d day%s ", + dt->days, pcmk__plural_s(dt->days)); } // At least print seconds (and optionally usecs) - if ((offset == 0) || (dt->seconds != 0) || (show_usec && (usec != 0))) { + if ((buf->len == 0) || (dt->seconds != 0) || (show_usec && (usec != 0))) { if (show_usec) { - sec_usec_as_string(dt->seconds, usec, result, &offset); + sec_usec_as_string(dt->seconds, usec, buf); } else { - offset += snprintf(result + offset, DATE_MAX - offset, "%d", - dt->seconds); + g_string_append_printf(buf, "%d", dt->seconds); } - offset += snprintf(result + offset, DATE_MAX - offset, " second%s", - pcmk__plural_s(dt->seconds)); + g_string_append_printf(buf, " second%s", pcmk__plural_s(dt->seconds)); } // More than one minute, so provide a more readable breakdown into units @@ -546,32 +544,37 @@ crm_duration_as_string(const crm_time_t *dt, int usec, bool show_usec, crm_time_get_sec(dt->seconds, &h, &m, &s); print_sec_component = ((s != 0) || (show_usec && (u != 0))); - offset += snprintf(result + offset, DATE_MAX - offset, " ("); + g_string_append(buf, " ("); if (h) { - offset += snprintf(result + offset, DATE_MAX - offset, - "%" PRIu32 " hour%s%s", h, pcmk__plural_s(h), - ((m != 0) || print_sec_component)? " " : ""); + g_string_append_printf(buf, "%" PRIu32 " hour%s", + h, pcmk__plural_s(h)); + + if ((m != 0) || print_sec_component) { + g_string_append_c(buf, ' '); + } } if (m) { - offset += snprintf(result + offset, DATE_MAX - offset, - "%" PRIu32 " minute%s%s", m, pcmk__plural_s(m), - print_sec_component? " " : ""); + g_string_append_printf(buf, "%" PRIu32 " minute%s", + m, pcmk__plural_s(m)); + + if (print_sec_component) { + g_string_append_c(buf, ' '); + } } if (print_sec_component) { if (show_usec) { - sec_usec_as_string(s, u, result, &offset); + sec_usec_as_string(s, u, buf); } else { - offset += snprintf(result + offset, DATE_MAX - offset, - "%" PRIu32, s); + g_string_append_printf(buf, "%" PRIu32, s); } - offset += snprintf(result + offset, DATE_MAX - offset, " second%s", - pcmk__plural_s(dt->seconds)); + g_string_append_printf(buf, " second%s", + pcmk__plural_s(dt->seconds)); } - offset += snprintf(result + offset, DATE_MAX - offset, ")"); + g_string_append_c(buf, ')'); } } @@ -579,35 +582,36 @@ crm_duration_as_string(const crm_time_t *dt, int usec, bool show_usec, * \internal * \brief Get a string representation of a time object * - * \param[in] dt Time to convert to string - * \param[in] usec Microseconds to add to \p dt - * \param[in] flags Group of \p crm_time_* string format options - * \param[out] result Where to store the result string + * \param[in] dt Time to convert to string + * \param[in] usec Microseconds to add to \p dt + * \param[in] flags Group of \c crm_time_* string format options * - * \note \p result must be of size \p DATE_MAX or larger. + * \return Newly allocated string representation of \p dt plus \p usec + * + * \note The caller is responsible for freeing the return value using \c free(). */ -static void -time_as_string_common(const crm_time_t *dt, int usec, uint32_t flags, - char *result) +static char * +time_as_string_common(const crm_time_t *dt, int usec, uint32_t flags) { crm_time_t *utc = NULL; - size_t offset = 0; + GString *buf = NULL; + char *result = NULL; if (!crm_time_is_defined(dt)) { - strcpy(result, ""); - return; + return pcmk__str_copy(""); } pcmk__assert(valid_sec_usec(dt->seconds, usec)); + buf = g_string_sized_new(128); + /* Simple cases: as duration, seconds, or seconds since epoch. * These never depend on time zone. */ if (pcmk_is_set(flags, crm_time_log_duration)) { - crm_duration_as_string(dt, usec, pcmk_is_set(flags, crm_time_usecs), - result); - return; + duration_as_string(dt, usec, pcmk_is_set(flags, crm_time_usecs), buf); + goto done; } if (pcmk_any_flags_set(flags, crm_time_seconds|crm_time_epoch)) { @@ -620,11 +624,11 @@ time_as_string_common(const crm_time_t *dt, int usec, uint32_t flags, } if (pcmk_is_set(flags, crm_time_usecs)) { - sec_usec_as_string(seconds, usec, result, &offset); + sec_usec_as_string(seconds, usec, buf); } else { - snprintf(result, DATE_MAX, "%lld", seconds); + g_string_append_printf(buf, "%lld", seconds); } - return; + goto done; } // Convert to UTC if local timezone was not requested @@ -643,9 +647,9 @@ time_as_string_common(const crm_time_t *dt, int usec, uint32_t flags, uint32_t d = 0; if (crm_time_get_isoweek(dt, &y, &w, &d)) { - offset += snprintf(result + offset, DATE_MAX - offset, - "%" PRIu32 "-W%.2" PRIu32 "-%" PRIu32, - y, w, d); + g_string_append_printf(buf, + "%" PRIu32 "-W%.2" PRIu32 "-%" PRIu32, + y, w, d); } } else if (pcmk_is_set(flags, crm_time_ordinal)) { // YYYY-DDD @@ -653,8 +657,7 @@ time_as_string_common(const crm_time_t *dt, int usec, uint32_t flags, uint32_t d = 0; if (crm_time_get_ordinal(dt, &y, &d)) { - offset += snprintf(result + offset, DATE_MAX - offset, - "%" PRIu32 "-%.3" PRIu32, y, d); + g_string_append_printf(buf, "%" PRIu32 "-%.3" PRIu32, y, d); } } else { // YYYY-MM-DD @@ -663,9 +666,9 @@ time_as_string_common(const crm_time_t *dt, int usec, uint32_t flags, uint32_t d = 0; if (crm_time_get_gregorian(dt, &y, &m, &d)) { - offset += snprintf(result + offset, DATE_MAX - offset, - "%.4" PRIu32 "-%.2" PRIu32 "-%.2" PRIu32, - y, m, d); + g_string_append_printf(buf, + "%.4" PRIu32 "-%.2" PRIu32 "-%.2" PRIu32, + y, m, d); } } } @@ -673,33 +676,36 @@ time_as_string_common(const crm_time_t *dt, int usec, uint32_t flags, if (pcmk_is_set(flags, crm_time_log_timeofday)) { uint32_t h = 0, m = 0, s = 0; - if (offset > 0) { - offset += snprintf(result + offset, DATE_MAX - offset, " "); + if (buf->len > 0) { + g_string_append_c(buf, ' '); } if (crm_time_get_timeofday(dt, &h, &m, &s)) { - offset += snprintf(result + offset, DATE_MAX - offset, - "%.2" PRIu32 ":%.2" PRIu32 ":%.2" PRIu32, - h, m, s); + g_string_append_printf(buf, + "%.2" PRIu32 ":%.2" PRIu32 ":%.2" PRIu32, + h, m, s); if (pcmk_is_set(flags, crm_time_usecs)) { - offset += snprintf(result + offset, DATE_MAX - offset, - ".%06" PRIu32, QB_ABS(usec)); + g_string_append_printf(buf, ".%06" PRIu32, QB_ABS(usec)); } } if (pcmk_is_set(flags, crm_time_log_with_timezone) && (dt->offset != 0)) { crm_time_get_sec(dt->offset, &h, &m, &s); - offset += snprintf(result + offset, DATE_MAX - offset, - " %c%.2" PRIu32 ":%.2" PRIu32, - ((dt->offset < 0)? '-' : '+'), h, m); + g_string_append_printf(buf, " %c%.2" PRIu32 ":%.2" PRIu32, + ((dt->offset < 0)? '-' : '+'), h, m); + } else { - offset += snprintf(result + offset, DATE_MAX - offset, "Z"); + g_string_append_c(buf, 'Z'); } } +done: crm_time_free(utc); + result = pcmk__str_copy(buf->str); + g_string_free(buf, TRUE); + return result; } /*! @@ -713,10 +719,7 @@ time_as_string_common(const crm_time_t *dt, int usec, uint32_t flags, char * crm_time_as_string(const crm_time_t *dt, int flags) { - char result[DATE_MAX] = { '\0', }; - - time_as_string_common(dt, 0, flags, result); - return pcmk__str_copy(result); + return time_as_string_common(dt, 0, flags); } /*! @@ -1901,24 +1904,6 @@ crm_time_add_years(crm_time_t * a_time, int extra) } } -static void -ha_get_tm_time(struct tm *target, const crm_time_t *source) -{ - *target = (struct tm) { - .tm_year = source->years - 1900, - .tm_mday = source->days, - .tm_sec = source->seconds % 60, - .tm_min = ( source->seconds / 60 ) % 60, - .tm_hour = source->seconds / HOUR_SECONDS, - .tm_isdst = -1, /* don't adjust */ - -#if defined(HAVE_STRUCT_TM_TM_GMTOFF) - .tm_gmtoff = source->offset -#endif - }; - mktime(target); -} - /* The high-resolution variant of time object was added to meet an immediate * need, and is kept internal API. * @@ -1927,44 +1912,23 @@ ha_get_tm_time(struct tm *target, const crm_time_t *source) * crm_time_t, pcmk__time_hr_t, and struct timespec (in lrmd_cmd_t). */ -pcmk__time_hr_t * -pcmk__time_hr_convert(pcmk__time_hr_t *target, const crm_time_t *dt) +static pcmk__time_hr_t * +time_to_hr(const crm_time_t *dt) { pcmk__time_hr_t *hr_dt = NULL; - if (dt) { - hr_dt = target; - if (hr_dt == NULL) { - hr_dt = pcmk__assert_alloc(1, sizeof(pcmk__time_hr_t)); - } - - *hr_dt = (pcmk__time_hr_t) { - .years = dt->years, - .months = dt->months, - .days = dt->days, - .seconds = dt->seconds, - .offset = dt->offset, - .duration = dt->duration - }; - } + pcmk__assert(dt != NULL); + hr_dt = pcmk__assert_alloc(1, sizeof(pcmk__time_hr_t)); + hr_dt->years = dt->years; + hr_dt->months = dt->months; + hr_dt->days = dt->days; + hr_dt->seconds = dt->seconds; + hr_dt->offset = dt->offset; + hr_dt->duration = dt->duration; return hr_dt; } -void -pcmk__time_set_hr_dt(crm_time_t *target, const pcmk__time_hr_t *hr_dt) -{ - pcmk__assert((target != NULL) && (hr_dt != NULL)); - *target = (crm_time_t) { - .years = hr_dt->years, - .months = hr_dt->months, - .days = hr_dt->days, - .seconds = hr_dt->seconds, - .offset = hr_dt->offset, - .duration = hr_dt->duration - }; -} - /*! * \internal * \brief Return the current time as a high-resolution time @@ -1985,10 +1949,8 @@ pcmk__time_hr_now(time_t *epoch) *epoch = tv.tv_sec; } crm_time_set_timet(&dt, &(tv.tv_sec)); - hr = pcmk__time_hr_convert(NULL, &dt); - if (hr != NULL) { - hr->useconds = tv.tv_nsec / QB_TIME_NS_IN_USEC; - } + hr = time_to_hr(&dt); + hr->useconds = tv.tv_nsec / QB_TIME_NS_IN_USEC; return hr; } @@ -2000,10 +1962,9 @@ pcmk__time_hr_new(const char *date_time) if (date_time == NULL) { hr_dt = pcmk__time_hr_now(NULL); } else { - crm_time_t *dt; + crm_time_t *dt = parse_date(date_time); - dt = parse_date(date_time); - hr_dt = pcmk__time_hr_convert(NULL, dt); + hr_dt = time_to_hr(dt); crm_time_free(dt); } return hr_dt; @@ -2015,41 +1976,132 @@ pcmk__time_hr_free(pcmk__time_hr_t * hr_dt) free(hr_dt); } +static void +ha_get_tm_time(struct tm *target, const pcmk__time_hr_t *source) +{ + *target = (struct tm) { + .tm_year = source->years - 1900, + + /* source->days is day of year, but we assign it to tm_mday instead of + * tm_yday. mktime() fixes it. See the mktime(3) man page for details. + */ + .tm_mday = source->days, + + // mktime() converts this to hours/minutes/seconds appropriately + .tm_sec = source->seconds, + + // Don't adjust DST here; let mktime() try to determine DST status + .tm_isdst = -1, + +#if defined(HAVE_STRUCT_TM_TM_GMTOFF) + .tm_gmtoff = source->offset +#endif + }; + mktime(target); +} + /*! * \internal - * \brief Expand a date/time format string, including %N for nanoseconds + * \brief Convert a struct tm to a \c GDateTime + * + * \param[in] tm Time object to convert + * \param[in] offset Offset from UTC (in seconds) * - * \param[in] format Date/time format string as per strftime(3) with the - * addition of %N for nanoseconds + * \return Newly allocated \c GDateTime object corresponding to \p tm, or + * \c NULL on error + * + * \note The caller is responsible for freeing the return value using + * \c g_date_time_unref(). + */ +static GDateTime * +get_g_date_time(const struct tm *tm, int offset) +{ + // Accept an offset argument in case tm lacks a tm_gmtoff member + char buf[sizeof("+hh:mm")] = { '\0', }; + const char *offset_s = NULL; + + GTimeZone *tz = NULL; + GDateTime *dt = NULL; + + if (QB_ABS(offset) <= DAY_SECONDS) { + uint32_t hours = 0; + uint32_t minutes = 0; + uint32_t seconds = 0; + int rc = 0; + + crm_time_get_sec(offset, &hours, &minutes, &seconds); + + rc = snprintf(buf, sizeof(buf), "%c%02" PRIu32 ":%02" PRIu32, + ((offset >= 0)? '+' : '-'), hours, minutes); + pcmk__assert(rc == (sizeof(buf) - 1)); + offset_s = buf; + + } else { + // offset out of range; use NULL as offset_s + CRM_LOG_ASSERT(QB_ABS(offset) <= DAY_SECONDS); + } + + /* @FIXME @COMPAT As of glib 2.68, g_time_zone_new() is deprecated in favor + * of g_time_zone_new_identifier(). However, calling + * g_time_zone_new_identifier() results in compiler warnings, even on a + * system with glib 2.84 installed. It is unclear why. + * + * The *_new_identifier() function was added (and the *_new() function + * deprecated) in version 2.68. They have the same signature. Ideally, we + * would choose which function to call here and below based the installed + * glib version using a CPP guard. + */ + tz = g_time_zone_new(offset_s); + dt = g_date_time_new(tz, tm->tm_year + 1900, tm->tm_mon + 1, tm->tm_mday, + tm->tm_hour, tm->tm_min, tm->tm_sec); + g_time_zone_unref(tz); + + return dt; +} + +/*! + * \internal + * \brief Expand a date/time format string, with support for fractional seconds + * + * \param[in] format Date/time format string compatible with + * \c g_date_time_format(), with additional support for + * \c "%N" for fractional seconds * \param[in] hr_dt Time value to format * - * \return Newly allocated string with formatted string + * \return Newly allocated string with formatted string, or \c NULL on error + * + * \note This function falls back to trying \c strftime() with a fixed-size + * buffer if \c g_date_time_format() fails. This fallback will be removed + * in a future release. */ char * pcmk__time_format_hr(const char *format, const pcmk__time_hr_t *hr_dt) { int scanned_pos = 0; // How many characters of format have been parsed int printed_pos = 0; // How many characters of format have been processed - size_t date_len = 0; - - char nano_s[10] = { '\0', }; - char date_s[128] = { '\0', }; + GString *buf = NULL; + char *result = NULL; struct tm tm = { 0, }; - crm_time_t dt = { 0, }; + GDateTime *gdt = NULL; if (format == NULL) { return NULL; } - pcmk__time_set_hr_dt(&dt, hr_dt); - ha_get_tm_time(&tm, &dt); - sprintf(nano_s, "%06d000", hr_dt->useconds); + + buf = g_string_sized_new(128); + + ha_get_tm_time(&tm, hr_dt); + gdt = get_g_date_time(&tm, hr_dt->offset); + if (gdt == NULL) { + goto done; + } while (format[scanned_pos] != '\0') { - int fmt_pos; // Index after last character to pass as-is - int nano_digits = 0; // Length of %N field width (if any) - char *tmp_fmt_s = NULL; - size_t nbytes = 0; + int fmt_pos = 0; // Index after last character to pass as-is + int frac_digits = 0; // %N specifier's width field value (if any) + gchar *tmp_fmt_s = NULL; + gchar *date_s = NULL; // Look for next format specifier const char *mark_s = strchr(&format[scanned_pos], '%'); @@ -2062,7 +2114,7 @@ pcmk__time_format_hr(const char *format, const pcmk__time_hr_t *hr_dt) } else { fmt_pos = mark_s - format; // Index of % - // Skip % and any field width + // Skip % and any width field scanned_pos = fmt_pos + 1; while (isdigit(format[scanned_pos])) { scanned_pos++; @@ -2074,12 +2126,26 @@ pcmk__time_format_hr(const char *format, const pcmk__time_hr_t *hr_dt) break; case 'N': // %[width]N + /* Fractional seconds. This was supposed to represent + * nanoseconds. However, we only store times at microsecond + * resolution, and the width field support makes this a + * general fractional component specifier rather than a + * nanoseconds specifier. + * + * Further, since we cap the width at 6 digits, a user + * cannot display times at greater than microsecond + * resolution. + * + * A leading zero in the width field is ignored, not treated + * as "use zero-padding." For example, "%03N" and "%3N" + * produce the same result. + */ scanned_pos++; - // Parse field width - nano_digits = atoi(&format[fmt_pos + 1]); - nano_digits = QB_MAX(nano_digits, 0); - nano_digits = QB_MIN(nano_digits, 6); + // Parse width field + frac_digits = atoi(&format[fmt_pos + 1]); + frac_digits = QB_MAX(frac_digits, 0); + frac_digits = QB_MIN(frac_digits, 6); break; default: // Some other specifier @@ -2091,47 +2157,80 @@ pcmk__time_format_hr(const char *format, const pcmk__time_hr_t *hr_dt) } } - if (date_len >= sizeof(date_s)) { - return NULL; // No room for remaining string - } + tmp_fmt_s = g_strndup(&format[printed_pos], fmt_pos - printed_pos); + date_s = g_date_time_format(gdt, tmp_fmt_s); - tmp_fmt_s = strndup(&format[printed_pos], fmt_pos - printed_pos); - if (tmp_fmt_s == NULL) { - return NULL; - } + if (date_s == NULL) { + char compat_date_s[1024] = { '\0' }; + size_t nbytes = 0; + + // @COMPAT Drop this fallback + crm_warn("Could not format time using format string '%s' with " + "g_date_time_format(); trying strftime(). In a future " + "release, use of strftime() as a fallback will be removed", + format); #ifdef HAVE_FORMAT_NONLITERAL #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wformat-nonliteral" -#endif - nbytes = strftime(&date_s[date_len], sizeof(date_s) - date_len, - tmp_fmt_s, &tm); +#endif // HAVE_FORMAT_NONLITERAL + nbytes = strftime(compat_date_s, sizeof(compat_date_s), tmp_fmt_s, + &tm); #ifdef HAVE_FORMAT_NONLITERAL #pragma GCC diagnostic pop -#endif - free(tmp_fmt_s); - if (nbytes == 0) { // Would overflow buffer - return NULL; +#endif // HAVE_FORMAT_NONLITERAL + + if (nbytes == 0) { + // Truncation, empty string, or error; impossible to discern + crm_err("Could not format time using format string '%s'", + format); + + // Ensure we return NULL + g_string_truncate(buf, 0); + g_free(tmp_fmt_s); + goto done; + } + date_s = g_strdup(compat_date_s); } - date_len += nbytes; + + g_string_append(buf, date_s); + g_free(date_s); + g_free(tmp_fmt_s); + printed_pos = scanned_pos; - if (nano_digits != 0) { - int nc = 0; - if (date_len >= sizeof(date_s)) { - return NULL; // No room to add nanoseconds - } - nc = snprintf(&date_s[date_len], sizeof(date_s) - date_len, - "%.*s", nano_digits, nano_s); + if (frac_digits != 0) { + // Descending powers of 10 (10^5 down to 10^0) + static const int powers[6] = { 1e5, 1e4, 1e3, 1e2, 1e1, 1e0 }; - if ((nc < 0) || (nc == (sizeof(date_s) - date_len))) { - return NULL; // Error or would overflow buffer - } - date_len += nc; + // Sanity check to ensure array access is in bounds + pcmk__assert((frac_digits > 0) && (frac_digits <= 6)); + + /* Append fractional seconds at the requested resolution, truncated + * toward zero. We're basically converting from microseconds to + * another unit here. For example, suppose the width field + * (frac_digits) is 3. This means "use millisecond resolution." Then + * we need to divide our microseconds value by 10^3, which is + * powers[3 - 1]. + * + * If the width field is 6 (microsecond resolution), then we divide + * our microseconds value by 10^0 == 1, which is powers[6 - 1]. + */ + g_string_append_printf(buf, "%0*d", frac_digits, + hr_dt->useconds / powers[frac_digits - 1]); } } - return (date_len == 0)? NULL : pcmk__str_copy(date_s); +done: + if (buf->len > 0) { + result = pcmk__str_copy(buf->str); + } + g_string_free(buf, TRUE); + + if (gdt != NULL) { + g_date_time_unref(gdt); + } + return result; } /*! @@ -2184,15 +2283,13 @@ pcmk__timespec2str(const struct timespec *ts, uint32_t flags) { struct timespec tmp_ts; crm_time_t dt; - char result[DATE_MAX] = { 0 }; if (ts == NULL) { qb_util_timespec_from_epoch_get(&tmp_ts); ts = &tmp_ts; } crm_time_set_timet(&dt, &ts->tv_sec); - time_as_string_common(&dt, ts->tv_nsec / QB_TIME_NS_IN_USEC, flags, result); - return pcmk__str_copy(result); + return time_as_string_common(&dt, ts->tv_nsec / QB_TIME_NS_IN_USEC, flags); } /*! @@ -2203,8 +2300,8 @@ pcmk__timespec2str(const struct timespec *ts, uint32_t flags) * * \return Readable version of \p interval_ms * - * \note The return value is a pointer to static memory that will be - * overwritten by later calls to this function. + * \note The return value is a pointer to static memory that may be overwritten + * by later calls to this function. */ const char * pcmk__readable_interval(guint interval_ms) @@ -2215,41 +2312,43 @@ pcmk__readable_interval(guint interval_ms) #define MS_IN_D (MS_IN_H * 24) #define MAXSTR sizeof("..d..h..m..s...ms") static char str[MAXSTR]; - int offset = 0; + GString *buf = NULL; + + if (interval_ms == 0) { + return "0s"; + } + + buf = g_string_sized_new(128); - str[0] = '\0'; if (interval_ms >= MS_IN_D) { - offset += snprintf(str + offset, MAXSTR - offset, "%ud", - interval_ms / MS_IN_D); + g_string_append_printf(buf, "%ud", interval_ms / MS_IN_D); interval_ms -= (interval_ms / MS_IN_D) * MS_IN_D; } if (interval_ms >= MS_IN_H) { - offset += snprintf(str + offset, MAXSTR - offset, "%uh", - interval_ms / MS_IN_H); + g_string_append_printf(buf, "%uh", interval_ms / MS_IN_H); interval_ms -= (interval_ms / MS_IN_H) * MS_IN_H; } if (interval_ms >= MS_IN_M) { - offset += snprintf(str + offset, MAXSTR - offset, "%um", - interval_ms / MS_IN_M); + g_string_append_printf(buf, "%um", interval_ms / MS_IN_M); interval_ms -= (interval_ms / MS_IN_M) * MS_IN_M; } // Ns, N.NNNs, or NNNms if (interval_ms >= MS_IN_S) { - offset += snprintf(str + offset, MAXSTR - offset, "%u", - interval_ms / MS_IN_S); + g_string_append_printf(buf, "%u", interval_ms / MS_IN_S); interval_ms -= (interval_ms / MS_IN_S) * MS_IN_S; + if (interval_ms > 0) { - offset += snprintf(str + offset, MAXSTR - offset, ".%03u", - interval_ms); + g_string_append_printf(buf, ".%03u", interval_ms); } - (void) snprintf(str + offset, MAXSTR - offset, "s"); + g_string_append_c(buf, 's'); } else if (interval_ms > 0) { - (void) snprintf(str + offset, MAXSTR - offset, "%ums", interval_ms); - - } else if (str[0] == '\0') { - strcpy(str, "0s"); + g_string_append_printf(buf, "%ums", interval_ms); } + + pcmk__assert(buf->len < sizeof(str)); + strncpy(str, buf->str, sizeof(str) - 1); + g_string_free(buf, TRUE); return str; } diff --git a/lib/common/logging.c b/lib/common/logging.c index 7ba407721ee..5fa8cd20c6c 100644 --- a/lib/common/logging.c +++ b/lib/common/logging.c @@ -1,5 +1,5 @@ /* - * Copyright 2004-2024 the Pacemaker project contributors + * Copyright 2004-2025 the Pacemaker project contributors * * The version control history for this file may have further details. * @@ -149,8 +149,6 @@ crm_log_deinit(void) pcmk__gthread_log_id = 0; } -#define FMT_MAX 256 - /*! * \internal * \brief Set the log format string based on the passed-in method @@ -174,30 +172,29 @@ set_format_string(int method, const char *daemon, pid_t use_pid, } else { // Everything else gets more detail, for advanced troubleshooting - - int offset = 0; - char fmt[FMT_MAX]; + GString *fmt = g_string_sized_new(256); if (method > QB_LOG_STDERR) { // If logging to file, prefix with timestamp, node name, daemon ID - offset += snprintf(fmt + offset, FMT_MAX - offset, - TIMESTAMP_FORMAT_SPEC " %s %-20s[%lu] ", - use_nodename, daemon, (unsigned long) use_pid); + g_string_append_printf(fmt, + TIMESTAMP_FORMAT_SPEC " %s %-20s[%lld] ", + use_nodename, daemon, (long long) use_pid); } // Add function name (in parentheses) - offset += snprintf(fmt + offset, FMT_MAX - offset, "(%%n"); + g_string_append(fmt, "(%n"); if (crm_tracing_enabled()) { // When tracing, add file and line number - offset += snprintf(fmt + offset, FMT_MAX - offset, "@%%f:%%l"); + g_string_append(fmt, "@%f:%l"); } - offset += snprintf(fmt + offset, FMT_MAX - offset, ")"); + g_string_append_c(fmt, ')'); // Add tag (if any), severity, and actual message - offset += snprintf(fmt + offset, FMT_MAX - offset, " %%g\t%%p: %%b"); + g_string_append(fmt, " %g\t%p: %b"); - CRM_LOG_ASSERT(offset > 0); - qb_log_format_set(method, fmt); + CRM_LOG_ASSERT(fmt->len > 0); + qb_log_format_set(method, fmt->str); + g_string_free(fmt, TRUE); } } @@ -521,7 +518,8 @@ crm_write_blackbox(int nsig, const struct qb_log_callsite *cs) static volatile int counter = 1; static volatile time_t last = 0; - char buffer[NAME_MAX]; + char *buffer = NULL; + int rc = 0; time_t now = time(NULL); if (blackbox_file_prefix == NULL) { @@ -538,7 +536,8 @@ crm_write_blackbox(int nsig, const struct qb_log_callsite *cs) return; } - snprintf(buffer, NAME_MAX, "%s.%d", blackbox_file_prefix, counter++); + buffer = crm_strdup_printf("%s.%d", blackbox_file_prefix, + counter++); if (nsig == SIGTRAP) { crm_notice("Blackbox dump requested, please see %s for contents", buffer); @@ -551,7 +550,13 @@ crm_write_blackbox(int nsig, const struct qb_log_callsite *cs) } last = now; - qb_log_blackbox_write_to_file(buffer); + + rc = qb_log_blackbox_write_to_file(buffer); + if (rc < 0) { + // System errno + crm_err("Failed to write blackbox file %s: %s", buffer, + strerror(-rc)); + } /* Flush the existing contents * A size change would also work @@ -570,6 +575,8 @@ crm_write_blackbox(int nsig, const struct qb_log_callsite *cs) raise(nsig); break; } + + free(buffer); } static const char * @@ -663,18 +670,22 @@ crm_log_filter(struct qb_log_callsite *cs) if (trace_tags != NULL) { uint32_t tag; - char token[500]; const char *offset = NULL; const char *next = trace_tags; + // @TODO Use g_strsplit() to simplify do { + char *token = NULL; + offset = next; next = strchrnul(offset, ','); - snprintf(token, sizeof(token), "%.*s", (int)(next - offset), offset); + token = crm_strdup_printf("%.*s", (int) (next - offset), offset); tag = g_quark_from_string(token); crm_info("Created GQuark %u from token '%s' in '%s'", tag, token, trace_tags); + free(token); + if (next[0] != 0) { next++; } diff --git a/lib/common/mock.c b/lib/common/mock.c index bc82d7836c3..2fb231ff3e1 100644 --- a/lib/common/mock.c +++ b/lib/common/mock.c @@ -399,7 +399,6 @@ __wrap_getpwnam_r(const char *name, struct passwd *pwd, char *buf, * preceded by: * * expect_*(__wrap_readlink, path[, ...]); - * expect_*(__wrap_readlink, buf[, ...]); * expect_*(__wrap_readlink, bufsize[, ...]); * will_return(__wrap_readlink, errno_to_set); * will_return(__wrap_readlink, link_contents); @@ -419,7 +418,6 @@ __wrap_readlink(const char *restrict path, char *restrict buf, const char *contents = NULL; check_expected_ptr(path); - check_expected(buf); check_expected(bufsize); errno = mock_type(int); contents = mock_ptr_type(const char *); diff --git a/lib/common/options.c b/lib/common/options.c index b8f4943fda7..cbcb0204eb4 100644 --- a/lib/common/options.c +++ b/lib/common/options.c @@ -1073,42 +1073,31 @@ static const pcmk__cluster_option_t primitive_meta[] = { * \internal * \brief Get the value of a Pacemaker environment variable option * - * If an environment variable option is set, with either a PCMK_ or (for - * backward compatibility) HA_ prefix, log and return the value. + * If an environment variable option is set, with either a \c "PCMK_" or (for + * backward compatibility) \c "HA_" prefix, log and return the value. * * \param[in] option Environment variable name (without prefix) * - * \return Value of environment variable option, or NULL in case of - * option name too long or value not found + * \return Value of environment variable, or \c NULL if not set */ const char * pcmk__env_option(const char *option) { - const char *const prefixes[] = {"PCMK_", "HA_"}; - char env_name[NAME_MAX]; - const char *value = NULL; + // @COMPAT Drop support for "HA_" options eventually + static const char *const prefixes[] = { "PCMK", "HA" }; CRM_CHECK(!pcmk__str_empty(option), return NULL); for (int i = 0; i < PCMK__NELEM(prefixes); i++) { - int rv = snprintf(env_name, NAME_MAX, "%s%s", prefixes[i], option); - - if (rv < 0) { - crm_err("Failed to write %s%s to buffer: %s", prefixes[i], option, - strerror(errno)); - return NULL; - } - - if (rv >= sizeof(env_name)) { - crm_trace("\"%s%s\" is too long", prefixes[i], option); - continue; - } + char *env_name = crm_strdup_printf("%s_%s", prefixes[i], option); + const char *value = getenv(env_name); - value = getenv(env_name); if (value != NULL) { crm_trace("Found %s = %s", env_name, value); + free(env_name); return value; } + free(env_name); } crm_trace("Nothing found for %s", option); @@ -1116,6 +1105,7 @@ pcmk__env_option(const char *option) } /*! + * \internal * \brief Set or unset a Pacemaker environment variable option * * Set an environment variable option with a \c "PCMK_" prefix and optionally @@ -1135,38 +1125,29 @@ void pcmk__set_env_option(const char *option, const char *value, bool compat) { // @COMPAT Drop support for "HA_" options eventually - const char *const prefixes[] = {"PCMK_", "HA_"}; - char env_name[NAME_MAX]; + static const char *const prefixes[] = { "PCMK", "HA" }; - CRM_CHECK(!pcmk__str_empty(option) && (strchr(option, '=') == NULL), - return); + CRM_CHECK(!pcmk__str_empty(option), return); for (int i = 0; i < PCMK__NELEM(prefixes); i++) { - int rv = snprintf(env_name, NAME_MAX, "%s%s", prefixes[i], option); - - if (rv < 0) { - crm_err("Failed to write %s%s to buffer: %s", prefixes[i], option, - strerror(errno)); - return; - } - - if (rv >= sizeof(env_name)) { - crm_trace("\"%s%s\" is too long", prefixes[i], option); - continue; - } + char *env_name = crm_strdup_printf("%s_%s", prefixes[i], option); + int rc = 0; if (value != NULL) { crm_trace("Setting %s to %s", env_name, value); - rv = setenv(env_name, value, 1); + rc = setenv(env_name, value, 1); } else { crm_trace("Unsetting %s", env_name); - rv = unsetenv(env_name); + rc = unsetenv(env_name); } - if (rv < 0) { - crm_err("Failed to %sset %s: %s", (value != NULL)? "" : "un", - env_name, strerror(errno)); + if (rc < 0) { + int err = errno; + + crm_err("Failed to %sset %s: %s", ((value != NULL)? "" : "un"), + env_name, strerror(err)); } + free(env_name); if (!compat && (value != NULL)) { // For set, don't proceed to HA_