[PATCH i-g-t 1/2] lib/igt_core: Move all config-related parsing to common_init_config

[PATCH i-g-t 1/2] lib/igt_core: Move all config-related parsing to common_init_config

[Paul Kocialkowski]

This moves all the pieces related to config parsing to the dedicated
function for this purpose, renamed common_init_config for consistency.

It allows making the common_init function less big and more readable.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@linux.intel.com>
---
 lib/igt_core.c | 77 ++++++++++++++++++++++++++--------------------------------
 1 file changed, 35 insertions(+), 42 deletions(-)

diff --git a/lib/igt_core.c b/lib/igt_core.c
index 03d9a5bf..e25276fc 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -622,13 +622,35 @@ static void oom_adjust_for_doom(void)
 }
 
 #ifdef HAVE_GLIB
-static int config_parse(void)
+static void common_init_config(void)
 {
+	char *key_file_env = NULL;
+	char *key_file_loc = NULL;
 	GError *error = NULL;
-	int rc;
+	int ret;
 
-	if (!igt_key_file)
-		return 0;
+	/* Determine igt config path */
+	key_file_env = getenv("IGT_CONFIG_PATH");
+	if (key_file_env) {
+		key_file_loc = key_file_env;
+	} else {
+		key_file_loc = malloc(100);
+		snprintf(key_file_loc, 100, "%s/.igtrc", g_get_home_dir());
+	}
+
+	/* Load igt config file */
+	igt_key_file = g_key_file_new();
+	ret = g_key_file_load_from_file(igt_key_file, key_file_loc,
+					G_KEY_FILE_NONE, &error);
+	if (error && error->code == G_KEY_FILE_ERROR) {
+		g_error_free(error);
+		g_key_file_free(igt_key_file);
+		igt_key_file = NULL;
+
+		goto out;
+	}
+
+	g_clear_error(&error);
 
 	frame_dump_path = getenv("IGT_FRAME_DUMP_PATH");
 
@@ -639,19 +661,18 @@ static int config_parse(void)
 
 	g_clear_error(&error);
 
-	rc = g_key_file_get_integer(igt_key_file, "DUT", "SuspendResumeDelay",
-				    &error);
-	if (error && error->code == G_KEY_FILE_ERROR_INVALID_VALUE) {
-		g_error_free(error);
-		return -2;
-	}
+	ret = g_key_file_get_integer(igt_key_file, "DUT", "SuspendResumeDelay",
+				     &error);
+	assert(!error || error->code != G_KEY_FILE_ERROR_INVALID_VALUE);
 
 	g_clear_error(&error);
 
-	if (rc != 0)
-		igt_set_autoresume_delay(rc);
+	if (ret != 0)
+		igt_set_autoresume_delay(ret);
 
-	return 0;
+out:
+	if (!key_file_env && key_file_loc)
+		free(key_file_loc);
 }
 #endif
 
@@ -678,9 +699,6 @@ static int common_init(int *argc, char **argv,
 	int extra_opt_count;
 	int all_opt_count;
 	int ret = 0;
-	char *key_file_loc = NULL;
-	char *key_file_env = NULL;
-	GError *error = NULL;
 	const char *env;
 
 	if (!isatty(STDOUT_FILENO) || getenv("IGT_PLAIN_OUTPUT"))
@@ -802,36 +820,11 @@ static int common_init(int *argc, char **argv,
 		}
 	}
 
-	key_file_env = getenv("IGT_CONFIG_PATH");
-	if (key_file_env) {
-		key_file_loc = key_file_env;
-	} else {
-		key_file_loc = malloc(100);
-		snprintf(key_file_loc, 100, "%s/.igtrc", g_get_home_dir());
-	}
-
 #ifdef HAVE_GLIB
-	igt_key_file = g_key_file_new();
-	ret = g_key_file_load_from_file(igt_key_file, key_file_loc,
-					G_KEY_FILE_NONE, &error);
-	if (error && error->code == G_KEY_FILE_ERROR) {
-		g_error_free(error);
-		g_key_file_free(igt_key_file);
-		igt_key_file = NULL;
-		ret = -2;
-
-		goto out;
-	}
-
-	g_clear_error(&error);
-
-	ret = config_parse();
+	common_init_config();
 #endif
 
 out:
-	if (!key_file_env && key_file_loc)
-		free(key_file_loc);
-
 	free(short_opts);
 	free(combined_opts);
 
-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t 2/2] lib/igt_core: Split out env-related handling to common_init_env

[Paul Kocialkowski]

This moves the parts of the code doing env-related handling from
common_init to a new dedicated common_init_env function, making
common_init a bit more readable.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@linux.intel.com>
---
 lib/igt_core.c | 46 ++++++++++++++++++++++++++--------------------
 1 file changed, 26 insertions(+), 20 deletions(-)

diff --git a/lib/igt_core.c b/lib/igt_core.c
index e25276fc..c0488e94 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -652,8 +652,6 @@ static void common_init_config(void)
 
 	g_clear_error(&error);
 
-	frame_dump_path = getenv("IGT_FRAME_DUMP_PATH");
-
 	if (!frame_dump_path)
 		frame_dump_path = g_key_file_get_string(igt_key_file, "Common",
 							"FrameDumpPath",
@@ -676,6 +674,31 @@ out:
 }
 #endif
 
+static void common_init_env(void)
+{
+	const char *env;
+
+	if (!isatty(STDOUT_FILENO) || getenv("IGT_PLAIN_OUTPUT"))
+		__igt_plain_output = true;
+
+	if (!__igt_plain_output)
+		setlocale(LC_ALL, "");
+
+	env = getenv("IGT_LOG_LEVEL");
+	if (env) {
+		if (strcmp(env, "debug") == 0)
+			igt_log_level = IGT_LOG_DEBUG;
+		else if (strcmp(env, "info") == 0)
+			igt_log_level = IGT_LOG_INFO;
+		else if (strcmp(env, "warn") == 0)
+			igt_log_level = IGT_LOG_WARN;
+		else if (strcmp(env, "none") == 0)
+			igt_log_level = IGT_LOG_NONE;
+	}
+
+	frame_dump_path = getenv("IGT_FRAME_DUMP_PATH");
+}
+
 static int common_init(int *argc, char **argv,
 		       const char *extra_short_opts,
 		       const struct option *extra_long_opts,
@@ -699,25 +722,8 @@ static int common_init(int *argc, char **argv,
 	int extra_opt_count;
 	int all_opt_count;
 	int ret = 0;
-	const char *env;
-
-	if (!isatty(STDOUT_FILENO) || getenv("IGT_PLAIN_OUTPUT"))
-		__igt_plain_output = true;
-
-	if (!__igt_plain_output)
-		setlocale(LC_ALL, "");
 
-	env = getenv("IGT_LOG_LEVEL");
-	if (env) {
-		if (strcmp(env, "debug") == 0)
-			igt_log_level = IGT_LOG_DEBUG;
-		else if (strcmp(env, "info") == 0)
-			igt_log_level = IGT_LOG_INFO;
-		else if (strcmp(env, "warn") == 0)
-			igt_log_level = IGT_LOG_WARN;
-		else if (strcmp(env, "none") == 0)
-			igt_log_level = IGT_LOG_NONE;
-	}
+	common_init_env();
 
 	command_str = argv[0];
 	if (strrchr(command_str, '/'))
-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 1/2] lib/igt_core: Move all config-related parsing to common_init_config

[Lyude Paul]

For the whole series:

Reviewed-by: Lyude <lyude@redhat.com>

And pushed

On Thu, 2017-07-20 at 17:11 +0300, Paul Kocialkowski wrote:
> This moves all the pieces related to config parsing to the dedicated
> function for this purpose, renamed common_init_config for
> consistency.
> 
> It allows making the common_init function less big and more readable.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@linux.intel.com>
> ---
>  lib/igt_core.c | 77 ++++++++++++++++++++++++++--------------------
> ------------
>  1 file changed, 35 insertions(+), 42 deletions(-)
> 
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index 03d9a5bf..e25276fc 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -622,13 +622,35 @@ static void oom_adjust_for_doom(void)
>  }
>  
>  #ifdef HAVE_GLIB
> -static int config_parse(void)
> +static void common_init_config(void)
>  {
> +	char *key_file_env = NULL;
> +	char *key_file_loc = NULL;
>  	GError *error = NULL;
> -	int rc;
> +	int ret;
>  
> -	if (!igt_key_file)
> -		return 0;
> +	/* Determine igt config path */
> +	key_file_env = getenv("IGT_CONFIG_PATH");
> +	if (key_file_env) {
> +		key_file_loc = key_file_env;
> +	} else {
> +		key_file_loc = malloc(100);
> +		snprintf(key_file_loc, 100, "%s/.igtrc",
> g_get_home_dir());
> +	}
> +
> +	/* Load igt config file */
> +	igt_key_file = g_key_file_new();
> +	ret = g_key_file_load_from_file(igt_key_file, key_file_loc,
> +					G_KEY_FILE_NONE, &error);
> +	if (error && error->code == G_KEY_FILE_ERROR) {
> +		g_error_free(error);
> +		g_key_file_free(igt_key_file);
> +		igt_key_file = NULL;
> +
> +		goto out;
> +	}
> +
> +	g_clear_error(&error);
>  
>  	frame_dump_path = getenv("IGT_FRAME_DUMP_PATH");
>  
> @@ -639,19 +661,18 @@ static int config_parse(void)
>  
>  	g_clear_error(&error);
>  
> -	rc = g_key_file_get_integer(igt_key_file, "DUT",
> "SuspendResumeDelay",
> -				    &error);
> -	if (error && error->code == G_KEY_FILE_ERROR_INVALID_VALUE)
> {
> -		g_error_free(error);
> -		return -2;
> -	}
> +	ret = g_key_file_get_integer(igt_key_file, "DUT",
> "SuspendResumeDelay",
> +				     &error);
> +	assert(!error || error->code !=
> G_KEY_FILE_ERROR_INVALID_VALUE);
>  
>  	g_clear_error(&error);
>  
> -	if (rc != 0)
> -		igt_set_autoresume_delay(rc);
> +	if (ret != 0)
> +		igt_set_autoresume_delay(ret);
>  
> -	return 0;
> +out:
> +	if (!key_file_env && key_file_loc)
> +		free(key_file_loc);
>  }
>  #endif
>  
> @@ -678,9 +699,6 @@ static int common_init(int *argc, char **argv,
>  	int extra_opt_count;
>  	int all_opt_count;
>  	int ret = 0;
> -	char *key_file_loc = NULL;
> -	char *key_file_env = NULL;
> -	GError *error = NULL;
>  	const char *env;
>  
>  	if (!isatty(STDOUT_FILENO) || getenv("IGT_PLAIN_OUTPUT"))
> @@ -802,36 +820,11 @@ static int common_init(int *argc, char **argv,
>  		}
>  	}
>  
> -	key_file_env = getenv("IGT_CONFIG_PATH");
> -	if (key_file_env) {
> -		key_file_loc = key_file_env;
> -	} else {
> -		key_file_loc = malloc(100);
> -		snprintf(key_file_loc, 100, "%s/.igtrc",
> g_get_home_dir());
> -	}
> -
>  #ifdef HAVE_GLIB
> -	igt_key_file = g_key_file_new();
> -	ret = g_key_file_load_from_file(igt_key_file, key_file_loc,
> -					G_KEY_FILE_NONE, &error);
> -	if (error && error->code == G_KEY_FILE_ERROR) {
> -		g_error_free(error);
> -		g_key_file_free(igt_key_file);
> -		igt_key_file = NULL;
> -		ret = -2;
> -
> -		goto out;
> -	}
> -
> -	g_clear_error(&error);
> -
> -	ret = config_parse();
> +	common_init_config();
>  #endif
>  
>  out:
> -	if (!key_file_env && key_file_loc)
> -		free(key_file_loc);
> -
>  	free(short_opts);
>  	free(combined_opts);
>  
-- 
Cheers,
	Lyude
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx