Browse Source

refactor: Simplify the log hinter

Signed-off-by: Kai Krakow <kai@kaishome.de>
Kai Krakow 6 years ago
parent
commit
5396370e5d
4 changed files with 77 additions and 60 deletions
  1. 4 1
      daemon/gamemode-ioprio.c
  2. 32 40
      daemon/gamemode-sched.c
  3. 14 19
      daemon/gamemode.c
  4. 27 0
      daemon/logging.h

+ 4 - 1
daemon/gamemode-ioprio.c

@@ -114,7 +114,10 @@ void game_mode_apply_ioprio(const GameModeContext *self, const pid_t client)
 		int invalid_ioprio = ioprio;
 		ioprio = CLAMP(0, 7, ioprio);
 		if (ioprio != invalid_ioprio)
-			LOG_ERROR("IO priority value %d invalid, clamping to %d\n", invalid_ioprio, ioprio);
+			LOG_ONCE(ERROR,
+			         "IO priority value %d invalid, clamping to %d\n",
+			         invalid_ioprio,
+			         ioprio);
 
 		/* We support only IOPRIO_CLASS_BE as IOPRIO_CLASS_RT required CAP_SYS_ADMIN */
 		ioprio = IOPRIO_PRIO_VALUE(IOPRIO_CLASS_BE, ioprio);

+ 32 - 40
daemon/gamemode-sched.c

@@ -67,9 +67,10 @@ void game_mode_apply_renice(const GameModeContext *self, const pid_t client)
 	long int renice = 0;
 	config_get_renice_value(config, &renice);
 	if ((renice < 1) || (renice > 20)) {
-		LOG_ERROR("Invalid renice value '%ld' reset to default: %d.\n",
-		          renice,
-		          -NICE_DEFAULT_PRIORITY);
+		LOG_ONCE(ERROR,
+		         "Invalid renice value '%ld' reset to default: %d.\n",
+		         renice,
+		         -NICE_DEFAULT_PRIORITY);
 		renice = NICE_DEFAULT_PRIORITY;
 	} else {
 		renice = -renice;
@@ -81,12 +82,12 @@ void game_mode_apply_renice(const GameModeContext *self, const pid_t client)
 	if (getpriority(PRIO_PROCESS, (id_t)client) != 0) {
 		LOG_ERROR("Refused to renice client [%d]: already reniced\n", client);
 	} else if (setpriority(PRIO_PROCESS, (id_t)client, (int)renice)) {
-		LOG_ERROR(
-		    "Failed to renice client [%d], ignoring error condition: %s\n"
-		    "    -- Your user may not have permission to do this. Please read the docs\n"
-		    "    -- to learn how to adjust the pam limits.\n",
-		    client,
-		    strerror(errno));
+		LOG_HINTED(ERROR,
+		           "Failed to renice client [%d], ignoring error condition: %s\n",
+		           "    -- Your user may not have permission to do this. Please read the docs\n"
+		           "    -- to learn how to adjust the pam limits.\n",
+		           client,
+		           strerror(errno));
 	}
 }
 
@@ -114,40 +115,31 @@ void game_mode_apply_scheduling(const GameModeContext *self, const pid_t client)
 	if (!(strcmp(softrealtime, "off") == 0) && (enable_softrealtime)) {
 		const struct sched_param p = { .sched_priority = 0 };
 		if (sched_setscheduler(client, SCHED_ISO | SCHED_RESET_ON_FORK, &p)) {
-			const char *additional_message = "";
-			switch (errno) {
-			case EPERM: {
-				static int once = 0;
-				if (once++)
-					break;
-				additional_message =
-				    "    -- The error indicates that you may be running a resource management\n"
-				    "    -- daemon managing your game launcher and it leaks lower scheduling\n"
-				    "    -- classes into the games. This is likely a bug in the management daemon\n"
-				    "    -- and not a bug in GameMode, it should be reported upstream.\n"
-				    "    -- If unsure, please also look here:\n"
-				    "    -- https://github.com/FeralInteractive/gamemode/issues/68\n";
-				break;
-			}
-			case EINVAL: {
-				static int once = 0;
-				if (once++)
-					break;
-				additional_message =
-				    "    -- The error indicates that your kernel may not support this. If you\n"
-				    "    -- don't know what SCHED_ISO means, you can safely ignore this. If you\n"
-				    "    -- expected it to work, ensure you're running a kernel with MuQSS or\n"
-				    "    -- PDS scheduler.\n"
-				    "    -- For further technical reading on the topic start here:\n"
-				    "    -- https://lwn.net/Articles/720227/\n";
-				break;
-			}
-			}
+			const char *hint = "";
+			HINT_ONCE_ON(
+			    errno == EPERM,
+			    hint,
+			    "    -- The error indicates that you may be running a resource management\n"
+			    "    -- daemon managing your game launcher and it leaks lower scheduling\n"
+			    "    -- classes into the games. This is likely a bug in the management daemon\n"
+			    "    -- and not a bug in GameMode, it should be reported upstream.\n"
+			    "    -- If unsure, please also look here:\n"
+			    "    -- https://github.com/FeralInteractive/gamemode/issues/68\n");
+			HINT_ONCE_ON(
+			    errno == EINVAL,
+			    hint,
+			    "    -- The error indicates that your kernel may not support this. If you\n"
+			    "    -- don't know what SCHED_ISO means, you can safely ignore this. If you\n"
+			    "    -- expected it to work, ensure you're running a kernel with MuQSS or\n"
+			    "    -- PDS scheduler.\n"
+			    "    -- For further technical reading on the topic start here:\n"
+			    "    -- https://lwn.net/Articles/720227/\n");
 			LOG_ERROR(
-			    "Failed setting client [%d] into SCHED_ISO mode, ignoring error condition: %s\n%s",
+			    "Failed setting client [%d] into SCHED_ISO mode, ignoring error condition: %s\n"
+			    "%s",
 			    client,
 			    strerror(errno),
-			    additional_message);
+			    hint);
 		}
 	} else {
 		LOG_ERROR("Skipped setting client [%d] into SCHED_ISO mode: softrealtime setting is '%s'\n",

+ 14 - 19
daemon/gamemode.c

@@ -323,16 +323,12 @@ bool game_mode_context_register(GameModeContext *self, pid_t client)
 	pthread_rwlock_rdlock(&self->rwlock); // ensure our pointer is sane
 	const GameModeClient *existing = game_mode_context_has_client(self, client);
 	if (existing) {
-		static int once = 0;
-		const char *additional_message =
-		    (once++
-		         ? ""
-		         : "    -- This may happen due to using exec or shell wrappers. You may want to\n"
-		           "    -- blacklist this client so GameMode can see its final name here.\n");
-		LOG_ERROR("Addition requested for already known client %d [%s].\n%s",
-		          existing->pid,
-		          existing->executable,
-		          additional_message);
+		LOG_HINTED(ERROR,
+		           "Addition requested for already known client %d [%s].\n",
+		           "    -- This may happen due to using exec or shell wrappers. You may want to\n"
+		           "    -- blacklist this client so GameMode can see its final name here.\n",
+		           existing->pid,
+		           existing->executable);
 		pthread_rwlock_unlock(&self->rwlock);
 		goto error_cleanup;
 	}
@@ -423,15 +419,14 @@ bool game_mode_context_unregister(GameModeContext *self, pid_t client)
 	pthread_rwlock_unlock(&self->rwlock);
 
 	if (!found) {
-		static int once = 0;
-		const char *additional_message =
-		    (once++
-		         ? ""
-		         : "    -- The parent process probably forked and tries to unregister from the\n"
-		           "    -- wrong process now. We cannot work around this. This message will\n"
-		           "    -- likely be paired with a nearby 'Removing expired game' which means we\n"
-		           "    -- cleaned up properly (we will log this event).\n");
-		LOG_ERROR("Removal requested for unknown process [%d].\n%s", client, additional_message);
+		LOG_HINTED(
+		    ERROR,
+		    "Removal requested for unknown process [%d].\n",
+		    "    -- The parent process probably forked and tries to unregister from the wrong\n"
+		    "    -- process now. We cannot work around this. This message will likely be paired\n"
+		    "    -- with a nearby 'Removing expired game' which means we cleaned up properly\n"
+		    "    -- (we will log this event). This hint will be displayed only once.\n",
+		    client);
 		return false;
 	}
 

+ 27 - 0
daemon/logging.h

@@ -62,6 +62,13 @@ POSSIBILITY OF SUCH DAMAGE.
 		}                                                                                          \
 	} while (0)
 
+#define LOG_ONCE(type, ...)                                                                        \
+	do {                                                                                           \
+		static int __once = 0;                                                                     \
+		if (!__once++)                                                                             \
+			LOG_##type(__VA_ARGS__);                                                               \
+	} while (0)
+
 /* Fatal warnings trigger an exit */
 #define FATAL_ERRORNO(msg)                                                                         \
 	do {                                                                                           \
@@ -74,6 +81,26 @@ POSSIBILITY OF SUCH DAMAGE.
 		exit(EXIT_FAILURE);                                                                        \
 	} while (0)
 
+/* Hinting helpers */
+#define HINT_ONCE(name, hint)                                                                      \
+	do {                                                                                           \
+		static int __once = 0;                                                                     \
+		name = (!__once++ ? hint : "");                                                            \
+	} while (0)
+
+#define HINT_ONCE_ON(cond, ...)                                                                    \
+	do {                                                                                           \
+		if (cond)                                                                                  \
+			HINT_ONCE(__VA_ARGS__);                                                                \
+	} while (0);
+
+#define LOG_HINTED(type, msg, hint, ...)                                                           \
+	do {                                                                                           \
+		const char *__arg;                                                                         \
+		HINT_ONCE(__arg, hint);                                                                    \
+		LOG_##type(msg "%s", __VA_ARGS__, __arg);                                                  \
+	} while (0)
+
 /**
  * Control if and how how we use syslog
  */