Browse Source

Minor C cleanup (#27)

* Minor C cleanup

 - some symbols can be made static:
    1. set_gov_state
    2. everything in gamemode_client.h
 - daemonize() can also take a const char*, since the name is only
   passed to printf() or syslog()
 - prevent shadowing of variables
 - use explicit (void) as parameter-list more consistently
 - use some more const.
   Move cast to more appropriate place and document that execv() behaves
   as if args where of type const *char and we trust on that.
 - example: Just use main(void), which is also an acceptable ISO-C decl
 - example: Use stderr for errors

* Fix -Wold-style-declaration issue
Leonard 7 years ago
parent
commit
752d877196
8 changed files with 34 additions and 27 deletions
  1. 1 1
      daemon/cpugovctl.c
  2. 1 1
      daemon/daemonize.c
  3. 1 1
      daemon/daemonize.h
  4. 2 2
      daemon/governors-query.c
  5. 12 6
      daemon/governors.c
  6. 1 1
      daemon/logging.c
  7. 3 3
      example/main.c
  8. 13 12
      lib/gamemode_client.h

+ 1 - 1
daemon/cpugovctl.c

@@ -40,7 +40,7 @@ POSSIBILITY OF SUCH DAMAGE.
 /**
  * Sets all governors to a value
  */
-void set_gov_state(const char *value)
+static void set_gov_state(const char *value)
 {
 	char governors[MAX_GOVERNORS][MAX_GOVERNOR_LENGTH] = { { 0 } };
 	int num = fetch_governors(governors);

+ 1 - 1
daemon/daemonize.c

@@ -39,7 +39,7 @@ POSSIBILITY OF SUCH DAMAGE.
 /**
  * Helper to perform standard UNIX daemonization
  */
-void daemonize(char *name)
+void daemonize(const char *name)
 {
 	/* Initial fork */
 	pid_t pid = fork();

+ 1 - 1
daemon/daemonize.h

@@ -35,4 +35,4 @@ POSSIBILITY OF SUCH DAMAGE.
  * Attempt daemonization of the process.
  * If this fails, the process will exit
  */
-void daemonize(char *name);
+void daemonize(const char *name);

+ 2 - 2
daemon/governors-query.c

@@ -77,7 +77,7 @@ int fetch_governors(char governors[MAX_GOVERNORS][MAX_GOVERNOR_LENGTH])
 		}
 
 		/* Only add this governor if it is unique */
-		for (int i = 0; i < num_governors; i++) {
+		for (int j = 0; j < num_governors; j++) {
 			if (strncmp(fullpath, governors[i], MAX_GOVERNOR_LENGTH) == 0) {
 				continue;
 			}
@@ -95,7 +95,7 @@ int fetch_governors(char governors[MAX_GOVERNORS][MAX_GOVERNOR_LENGTH])
 /**
  * Return the current governor state
  */
-const char *get_gov_state()
+const char *get_gov_state(void)
 {
 	/* Cached primary governor state */
 	static char governor[64] = { 0 };

+ 12 - 6
daemon/governors.c

@@ -46,7 +46,7 @@ static const char *initial = NULL;
 /**
  * Cache the governor state as seen at startup
  */
-void update_initial_gov_state()
+void update_initial_gov_state(void)
 {
 	initial = get_gov_state();
 }
@@ -61,9 +61,9 @@ bool set_governors(const char *value)
 	int ret = 0;
 	int r = -1;
 
-	const char *govern = value ? value : initial;
-	char *exec_args[] = {
-		"/usr/bin/pkexec", LIBEXECDIR "/cpugovctl", "set", (char *)govern, NULL,
+	const char *const govern = value ? value : initial;
+	const char *const exec_args[] = {
+		"/usr/bin/pkexec", LIBEXECDIR "/cpugovctl", "set", govern, NULL,
 	};
 
 	LOG_MSG("Requesting update of governor policy to %s\n", govern);
@@ -73,7 +73,13 @@ bool set_governors(const char *value)
 		return false;
 	} else if (p == 0) {
 		/* Execute the command */
-		if ((r = execv(exec_args[0], exec_args)) != 0) {
+		/* Note about cast:
+		 *   The statement about argv[] and envp[] being constants is
+		 *   included to make explicit to future writers of language
+		 *   bindings that these objects are completely constant.
+		 * http://pubs.opengroup.org/onlinepubs/9699919799/functions/exec.html
+		 */
+		if ((r = execv(exec_args[0], (char *const *)exec_args)) != 0) {
 			LOG_ERROR("Failed to execute cpugovctl helper: %s %s\n", exec_args[1], strerror(errno));
 			exit(EXIT_FAILURE);
 		}
@@ -100,7 +106,7 @@ bool set_governors(const char *value)
 /**
  * Return the cached governor seen at startup
  */
-const char *get_initial_governor()
+const char *get_initial_governor(void)
 {
 	return initial;
 }

+ 1 - 1
daemon/logging.c

@@ -47,7 +47,7 @@ void set_use_syslog(const char *name)
 /**
  *  Simple getter for the syslog var
  */
-bool get_use_syslog()
+bool get_use_syslog(void)
 {
 	return use_syslog;
 }

+ 3 - 3
example/main.c

@@ -34,11 +34,11 @@ POSSIBILITY OF SUCH DAMAGE.
 #include <stdio.h>
 #include <unistd.h>
 
-int main(__attribute__((unused)) int argc, __attribute__((unused)) char **argv)
+int main(void)
 {
 	/* Request we start game mode */
 	if (gamemode_request_start() != 0) {
-		printf("Failed to request gamemode start: %s...\n", gamemode_error_string());
+		fprintf(stderr, "Failed to request gamemode start: %s...\n", gamemode_error_string());
 	}
 
 	/* Simulate running a game */
@@ -46,6 +46,6 @@ int main(__attribute__((unused)) int argc, __attribute__((unused)) char **argv)
 
 	/* Request we end game mode (optional) */
 	if (gamemode_request_end() != 0) {
-		printf("Failed to request gamemode end: %s...\n", gamemode_error_string());
+		fprintf(stderr, "Failed to request gamemode end: %s...\n", gamemode_error_string());
 	}
 }

+ 13 - 12
lib/gamemode_client.h

@@ -38,7 +38,7 @@ POSSIBILITY OF SUCH DAMAGE.
 #include <errno.h>
 #include <string.h>
 
-char _client_error_string[512] = { 0 };
+static char _client_error_string[512] = { 0 };
 
 /**
  * Load libgamemode dynamically to dislodge us from most dependencies.
@@ -46,7 +46,7 @@ char _client_error_string[512] = { 0 };
  * See SDL2 for an example of the reasoning behind this in terms of
  * dynamic versioning as well.
  */
-volatile int _libgamemode_loaded = 1;
+static volatile int _libgamemode_loaded = 1;
 
 /* Typedefs for the functions to load */
 typedef int (*_gamemode_request_start)(void);
@@ -54,18 +54,19 @@ typedef int (*_gamemode_request_end)(void);
 typedef const char *(*_gamemode_error_string)(void);
 
 /* Storage for functors */
-_gamemode_request_start _REAL_gamemode_request_start = NULL;
-_gamemode_request_end _REAL_gamemode_request_end = NULL;
-_gamemode_error_string _REAL_gamemode_error_string = NULL;
+static _gamemode_request_start _REAL_gamemode_request_start = NULL;
+static _gamemode_request_end _REAL_gamemode_request_end = NULL;
+static _gamemode_error_string _REAL_gamemode_error_string = NULL;
 
 /**
  * Internal helper to perform the symbol binding safely.
  *
  * Returns 0 on success and -1 on failure
  */
-__attribute__((always_inline)) inline int _bind_libgamemode_symbol(void *handle, const char *name,
-                                                                   void **out_func,
-                                                                   size_t func_size)
+__attribute__((always_inline)) static inline int _bind_libgamemode_symbol(void *handle,
+                                                                          const char *name,
+                                                                          void **out_func,
+                                                                          size_t func_size)
 {
 	void *symbol_lookup = NULL;
 	char *dl_error = NULL;
@@ -88,7 +89,7 @@ __attribute__((always_inline)) inline int _bind_libgamemode_symbol(void *handle,
  *
  * Returns 0 on success and -1 on failure
  */
-__attribute__((always_inline)) inline int _load_libgamemode(void)
+__attribute__((always_inline)) static inline int _load_libgamemode(void)
 {
 	/* We start at 1, 0 is a success and -1 is a fail */
 	if (_libgamemode_loaded != 1) {
@@ -146,7 +147,7 @@ __attribute__((always_inline)) inline int _load_libgamemode(void)
 /**
  * Redirect to the real libgamemode
  */
-__attribute__((always_inline)) inline const char *gamemode_error_string(void)
+__attribute__((always_inline)) static inline const char *gamemode_error_string(void)
 {
 	/* If we fail to load the system gamemode, return our error string */
 	if (_load_libgamemode() < 0) {
@@ -164,7 +165,7 @@ __attribute__((always_inline)) inline const char *gamemode_error_string(void)
 #ifdef GAMEMODE_AUTO
 __attribute__((constructor))
 #else
-__attribute__((always_inline)) inline
+__attribute__((always_inline)) static inline
 #endif
 int gamemode_request_start(void)
 {
@@ -190,7 +191,7 @@ int gamemode_request_start(void)
 #ifdef GAMEMODE_AUTO
 __attribute__((destructor))
 #else
-__attribute__((always_inline)) inline
+__attribute__((always_inline)) static inline
 #endif
 int gamemode_request_end(void)
 {