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

Marian Buschsieweke notifications at github.com
Fri Feb 22 23:33:56 CET 2019


maribu requested changes on this pull request.

I think the `enum` type now allows for some optimization of the checking. @kaspar030 indicated that compile time checks might be possible with the `enum`, but I personally do not know how to do this. (Except for using a `switch()` and let the compiler warn when not all enum values are covered.) But the run time check can be done a bit more efficient when assuming that the thread state is a valid enum value

> @@ -43,6 +43,28 @@ static const char *state_names[] = {
     [STATUS_COND_BLOCKED] = "bl cond",
 };
 
+#define STATE_NAMES_NUMOF ((int)(sizeof(state_names)/sizeof(state_names[0])))
+#define STATE_NAME_UNKNOWN "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.
+ */
+static const char *state_to_string(int state)

The type of `state` should now be `thread_state_t`

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

`STATUS_NUMOF` gixves the number of possible thread states. It would be possible to just set the number of array elements in `state_names` to `STATUS_NUMOF` so the compiler would initialize all not explicitly set elements with `NULL`. Then it can be (almost) safely assumed that `state_names[x]` will be either the correct state name or `NULL`, assuming that `x` is a valid enum value (and not `STATUS_NUMOF`). This assumption could simplify the `state_to_string()` function a bit

-- 
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-207054144
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190222/08a9944e/attachment-0001.html>


More information about the notifications mailing list