From 3de38e3991fc89141bdccb18b5fce32d5e7b2782 Mon Sep 17 00:00:00 2001
From: Utz-Uwe Haus <uhaus@cray.com>
Date: Tue, 2 Mar 2021 18:21:46 +0100
Subject: [PATCH] Ensure mstro_cdo_state_describe() uses ringbuffer
This fixes #121, where the state updates (which print two states in one
log statement) always got the second description wrong.
---
include/maestro/i_cdo.h | 9 ++++++++-
maestro/cdo.c | 20 ++++++++++++++++----
2 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/include/maestro/i_cdo.h b/include/maestro/i_cdo.h
index d4e2fbd4..93f20dab 100644
--- a/include/maestro/i_cdo.h
+++ b/include/maestro/i_cdo.h
@@ -156,7 +156,14 @@ typedef uint32_t mstro_cdo_state;
/** The NIL UUID */
#define MSTRO_CDO_ID_INVALID { .id = {0,0,0,0, 0,0,0,0, 0,0,0,0, 0,0,0,0 } }
-/** return a string describing the CDO state @arg s */
+/** return a string describing the CDO state @arg s
+ **
+ ** The string is owned by this function, but this function can safely
+ ** be called from multiple threads (the buffer is thread-local). The
+ ** buffer will be re-used by subsequent calls in the same thread
+ ** after a small number of invocations (currently: 3, see @ref
+ ** MAX_STATE_DESCRIPTION_BUFFERS).
+ **/
const char *
mstro_cdo_state_describe(mstro_cdo_state s);
diff --git a/maestro/cdo.c b/maestro/cdo.c
index 867151da..8ec685e4 100644
--- a/maestro/cdo.c
+++ b/maestro/cdo.c
@@ -1633,22 +1633,34 @@ struct {
/** a reasonably safe way to count the number of entries in an array of structures */
#define COUNT_OF(x) ((sizeof(x)/sizeof(0[x])) / ((size_t)(!(sizeof(x) % sizeof(0[x])))))
+
+/** we support this many concurrent state buffers in flight per
+ * thread. Hideous, but callers typically will use this function in a
+ * PRINT statement, with up to two states (to compare them) at most
+ */
+
+#define MAX_STATE_DESCRIPTION_BUFFERS 3
const char *
mstro_cdo_state_describe(mstro_cdo_state s)
{
- static _Thread_local char buf[sizeof(states_and_names)];
+ static _Thread_local char buf[MAX_STATE_DESCRIPTION_BUFFERS][sizeof(states_and_names)];
+ static _Thread_local size_t bufid = 0;
- buf[0]='\0';
+ // switch to new buffer
+ bufid = (bufid+1) % MAX_STATE_DESCRIPTION_BUFFERS;
+
+ buf[bufid][0]='\0';
if(s==MSTRO_CDO_STATE_INVALID)
return "INVALID";
else {
for(size_t i=0; i<COUNT_OF(states_and_names); i++) {
if(s&states_and_names[i].state) {
- strcat(buf, states_and_names[i].name);
+ strcat(buf[bufid], states_and_names[i].name);
}
}
- return buf+1; /* cut off leading '|' we put there for simplicity
+
+ return buf[bufid]+1; /* cut off leading '|' we put there for simplicity
* of the loop above */
}
}
--
GitLab