From ec8fcd69be73362046b8c43b6f981c2ce21d726e Mon Sep 17 00:00:00 2001 From: NiLuJe Date: Wed, 13 May 2020 17:05:05 -0400 Subject: [PATCH] Minor cosmetic tweaks to the KFMon list code (#24) * Switch to an enum for kfmon ipc errors codes. * Also switch to a case switch in nm_kfmon_return_handler. * Comment out the default case to have -Wswitch shout at you if you missed one. * Simplify pointer chasing loop. * Explicitly mention KFMON_IPC_EAGAIN in the return_handler. * Simplify some more. * Make GCC happy. * Slightly less convoluted. * Alphabetical header order. --- src/kfmon.c | 66 ++++++++++++++++++++++++++--------------------------- src/kfmon.h | 60 +++++++++++++++++++++++++----------------------- 2 files changed, 64 insertions(+), 62 deletions(-) diff --git a/src/kfmon.c b/src/kfmon.c index c2d9c81..f868078 100644 --- a/src/kfmon.c +++ b/src/kfmon.c @@ -1,18 +1,18 @@ #define _GNU_SOURCE #include +#include #include #include #include #include -#include -#include -#include #include #include +#include +#include -#include "util.h" -#include "kfmon_helpers.h" #include "kfmon.h" +#include "kfmon_helpers.h" +#include "util.h" // Free all resources allocated by a list and its nodes static void teardown_list(kfmon_watch_list_t* list) { @@ -405,11 +405,9 @@ int nm_kfmon_list_request(const char *restrict foo __attribute__((unused))) { NM_LOG("List has %zu nodes", list.count); NM_LOG("Head is at %p", list.head); NM_LOG("Tail is at %p", list.tail); - kfmon_watch_node_t* node = list.head; - while (node) { + for (kfmon_watch_node_t* node = list.head; node != NULL; node = node->next) { NM_LOG("Dumping node at %p", node); NM_LOG("idx: %hhu // filename: %s // label: %s", node->watch.idx, node->watch.filename, node->watch.label); - node = node->next; } // Destroy it @@ -421,61 +419,61 @@ int nm_kfmon_list_request(const char *restrict foo __attribute__((unused))) { } // Giant ladder of fail -nm_action_result_t* nm_kfmon_return_handler(int status, char **err_out) { +nm_action_result_t* nm_kfmon_return_handler(kfmon_ipc_errno_e status, char **err_out) { #define NM_ERR_RET NULL - if (status != EXIT_SUCCESS) { + switch (status) { + case KFMON_IPC_OK: + NM_RETURN_OK(nm_action_result_silent()); // Fail w/ the right log message - if (status == KFMON_IPC_ETIMEDOUT) { + case KFMON_IPC_ETIMEDOUT: NM_RETURN_ERR("Timed out waiting for KFMon"); - } else if (status == KFMON_IPC_EPIPE) { + case KFMON_IPC_EPIPE: NM_RETURN_ERR("KFMon closed the connection"); - } else if (status == KFMON_IPC_ENODATA) { + case KFMON_IPC_ENODATA: NM_RETURN_ERR("No more data to read"); - } else if (status == KFMON_IPC_READ_FAILURE) { + case KFMON_IPC_READ_FAILURE: // NOTE: Let's hope close() won't mangle errno... NM_RETURN_ERR("read: %m"); - } else if (status == KFMON_IPC_SEND_FAILURE) { + case KFMON_IPC_SEND_FAILURE: // NOTE: Let's hope close() won't mangle errno... NM_RETURN_ERR("send: %m"); - } else if (status == KFMON_IPC_SOCKET_FAILURE) { + case KFMON_IPC_SOCKET_FAILURE: NM_RETURN_ERR("Failed to create local KFMon IPC socket (socket: %m)"); - } else if (status == KFMON_IPC_CONNECT_FAILURE) { + case KFMON_IPC_CONNECT_FAILURE: NM_RETURN_ERR("KFMon IPC is down (connect: %m)"); - } else if (status == KFMON_IPC_POLL_FAILURE) { + case KFMON_IPC_POLL_FAILURE: // NOTE: Let's hope close() won't mangle errno... NM_RETURN_ERR("poll: %m"); - } else if (status == KFMON_IPC_CALLOC_FAILURE) { + case KFMON_IPC_CALLOC_FAILURE: NM_RETURN_ERR("calloc: %m"); - } else if (status == KFMON_IPC_REPLY_READ_FAILURE) { + case KFMON_IPC_REPLY_READ_FAILURE: // NOTE: Let's hope close() won't mangle errno... NM_RETURN_ERR("Failed to read KFMon's reply (%m)"); - } else if (status == KFMON_IPC_LIST_PARSE_FAILURE) { + case KFMON_IPC_LIST_PARSE_FAILURE: NM_RETURN_ERR("Failed to parse the list of watches (no separator found)"); - } else if (status == KFMON_IPC_ERR_INVALID_ID) { + case KFMON_IPC_ERR_INVALID_ID: NM_RETURN_ERR("Requested to start an invalid watch index"); - } else if (status == KFMON_IPC_ERR_INVALID_NAME) { + case KFMON_IPC_ERR_INVALID_NAME: NM_RETURN_ERR("Requested to trigger an invalid watch filename (expected the basename of the image trigger)"); - } else if (status == KFMON_IPC_WARN_ALREADY_RUNNING) { + case KFMON_IPC_WARN_ALREADY_RUNNING: NM_RETURN_ERR("Requested watch is already running"); - } else if (status == KFMON_IPC_WARN_SPAWN_BLOCKED) { + case KFMON_IPC_WARN_SPAWN_BLOCKED: NM_RETURN_ERR("A spawn blocker is currently running"); - } else if (status == KFMON_IPC_WARN_SPAWN_INHIBITED) { + case KFMON_IPC_WARN_SPAWN_INHIBITED: NM_RETURN_ERR("Spawns are currently inhibited"); - } else if (status == KFMON_IPC_ERR_REALLY_MALFORMED_CMD) { + case KFMON_IPC_ERR_REALLY_MALFORMED_CMD: NM_RETURN_ERR("KFMon couldn't parse our command"); - } else if (status == KFMON_IPC_ERR_MALFORMED_CMD) { + case KFMON_IPC_ERR_MALFORMED_CMD: NM_RETURN_ERR("Bad command syntax"); - } else if (status == KFMON_IPC_ERR_INVALID_CMD) { + case KFMON_IPC_ERR_INVALID_CMD: NM_RETURN_ERR("Command wasn't recognized by KFMon"); - } else if (status == KFMON_IPC_UNKNOWN_REPLY) { + case KFMON_IPC_UNKNOWN_REPLY: NM_RETURN_ERR("We couldn't make sense of KFMon's reply"); - } else { + case KFMON_IPC_EAGAIN: + default: // Should never happen NM_RETURN_ERR("Something went wrong"); - } - } else { - NM_RETURN_OK(nm_action_result_silent()); } #undef NM_ERR_RET diff --git a/src/kfmon.h b/src/kfmon.h index bc0842c..058094f 100644 --- a/src/kfmon.h +++ b/src/kfmon.h @@ -6,38 +6,42 @@ extern "C" { #include #include +#include #include "action.h" // Path to KFMon's IPC Unix socket #define KFMON_IPC_SOCKET "/tmp/kfmon-ipc.ctl" -// Flags for the failure bingo bitmask -#define KFMON_IPC_ETIMEDOUT (1 << 1) -#define KFMON_IPC_EPIPE (1 << 2) -#define KFMON_IPC_ENODATA (1 << 3) -// syscall failures -#define KFMON_IPC_READ_FAILURE (1 << 4) -#define KFMON_IPC_SEND_FAILURE (1 << 5) -#define KFMON_IPC_SOCKET_FAILURE (1 << 6) -#define KFMON_IPC_CONNECT_FAILURE (1 << 7) -#define KFMON_IPC_POLL_FAILURE (1 << 8) -#define KFMON_IPC_CALLOC_FAILURE (1 << 9) -#define KFMON_IPC_REPLY_READ_FAILURE (1 << 10) -#define KFMON_IPC_LIST_PARSE_FAILURE (1 << 11) -// Those match the actual string sent over the wire -#define KFMON_IPC_ERR_INVALID_ID (1 << 12) -#define KFMON_IPC_ERR_INVALID_NAME (1 << 13) -#define KFMON_IPC_WARN_ALREADY_RUNNING (1 << 14) -#define KFMON_IPC_WARN_SPAWN_BLOCKED (1 << 15) -#define KFMON_IPC_WARN_SPAWN_INHIBITED (1 << 16) -#define KFMON_IPC_ERR_REALLY_MALFORMED_CMD (1 << 17) -#define KFMON_IPC_ERR_MALFORMED_CMD (1 << 18) -#define KFMON_IPC_ERR_INVALID_CMD (1 << 19) -// Not an error ;p -//#define KFMON_IPC_OK -#define KFMON_IPC_UNKNOWN_REPLY (1 << 20) -// Not an error either, needs we have more to read... -#define KFMON_IPC_EAGAIN (1 << 21) +// Flags for the failure bingo +typedef enum { + // Not an error ;p + KFMON_IPC_OK = EXIT_SUCCESS, + // NOTE: Start > 256 to stay clear of errno + KFMON_IPC_ETIMEDOUT = 512, + KFMON_IPC_EPIPE, + KFMON_IPC_ENODATA, + // syscall failures + KFMON_IPC_READ_FAILURE, + KFMON_IPC_SEND_FAILURE, + KFMON_IPC_SOCKET_FAILURE, + KFMON_IPC_CONNECT_FAILURE, + KFMON_IPC_POLL_FAILURE, + KFMON_IPC_CALLOC_FAILURE, + KFMON_IPC_REPLY_READ_FAILURE, + KFMON_IPC_LIST_PARSE_FAILURE, + // Those match the actual string sent over the wire + KFMON_IPC_ERR_INVALID_ID, + KFMON_IPC_ERR_INVALID_NAME, + KFMON_IPC_WARN_ALREADY_RUNNING, + KFMON_IPC_WARN_SPAWN_BLOCKED, + KFMON_IPC_WARN_SPAWN_INHIBITED, + KFMON_IPC_ERR_REALLY_MALFORMED_CMD, + KFMON_IPC_ERR_MALFORMED_CMD, + KFMON_IPC_ERR_INVALID_CMD, + KFMON_IPC_UNKNOWN_REPLY, + // Not an error either, means we have more to read... + KFMON_IPC_EAGAIN, +} kfmon_ipc_errno_e; // A single watch item typedef struct { @@ -65,7 +69,7 @@ typedef struct { typedef int (*ipc_handler_t)(int, void *); // Given one of the error codes listed above, return properly from an action. Success is silent. -nm_action_result_t* nm_kfmon_return_handler(int error, char **err_out); +nm_action_result_t* nm_kfmon_return_handler(kfmon_ipc_errno_e status, char **err_out); // Send a simple KFMon IPC request, one where the reply is only used for its diagnostic value. int nm_kfmon_simple_request(const char *restrict ipc_cmd, const char *restrict ipc_arg);