[riot-notifications] [RIOT-OS/RIOT] sys/ps: Improve robustness against string table errors. (#10977)

Kaspar Schleiser notifications at github.com
Sat Feb 9 12:02:09 CET 2019


kaspar030 requested changes on this pull request.

seeing that the thread status names are plain integers 0..something, maybe it would make sense to change them from define to enum, with STATES_NUMOF as last. the names array could have it's sized pinned with that. maybe the compiler would even warn if an enum out of range is used.

I also don't like differing behaviour dependent on whether we're building with develhelp.

> @@ -42,6 +43,28 @@ static const char *state_names[] = {
     [STATUS_MBOX_BLOCKED] = "bl mbox",
 };
 
+#define N_STATE_NAMES ((int)(sizeof(state_names)/sizeof(state_names[0])))

please use STATE_NAMES_NUMOF.

> @@ -42,6 +43,28 @@ static const char *state_names[] = {
     [STATUS_MBOX_BLOCKED] = "bl mbox",
 };
 
+#define N_STATE_NAMES ((int)(sizeof(state_names)/sizeof(state_names[0])))
+#define UNKNOWN_STATE_NAME "unknown"

STATE_NAME_UNKNOWN?

> @@ -42,6 +43,28 @@ static const char *state_names[] = {
     [STATUS_MBOX_BLOCKED] = "bl mbox",
 };
 
+#define N_STATE_NAMES ((int)(sizeof(state_names)/sizeof(state_names[0])))
+#define UNKNOWN_STATE_NAME "unknown"
+
+/**
+ * Convert a thread state code to a human readable string.
+ *
+ * Use this function instead of a direct array lookup: if ever state_names
+ * and the actual states in tcb.h get out of sync, an invalid index can be
+ * generated, and direct access will cause a crash that this function is
+ * designed to prevent.
+ */
+const char *state_to_string(int state)

why int?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/10977#pullrequestreview-201859347
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190209/2a31fe1e/attachment.html>


More information about the notifications mailing list