Re: [igt-dev] [PATCH i-g-t] lib/igt_core: Don't return too early in common_init_config

["Dixit, Ashutosh" <ashutosh.dixit@intel.com>]

On Mon, 26 Oct 2020 01:48:40 -0700, Petri Latvala wrote:
>
> common_init_config is responsible for adding the device filters
> supplied through the environment to the filter list. If the .igtrc
> file cannot be opened, make sure the filter is added still.
>
> Signed-off-by: Petri Latvala <petri.latvala@intel.com>
> Cc: Arkadiusz Hiler <arek@hiler.eu>
> ---
>  lib/igt_core.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index b358173f..8f9d925b 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -775,21 +775,20 @@ GKeyFile *igt_load_igtrc(void)
>  static void common_init_config(void)
>  {
>	GError *error = NULL;
> -	int ret;
> +	int ret = 0;
>
>	igt_key_file = igt_load_igtrc();
> -	if (!igt_key_file)
> -		return;

I believe the only change needed is the one above. The remaining
igt_key_file checks below are not needed because it looks like if we pass
NULL igt_key_file g_key_file_get_string will return NULL and
g_key_file_get_integer will return 0 (that is previously initialized values
are undisturbed).

At the link below it is not clearly stated what happens when GKeyFile is
NULL so I am mostly inferring from the way it is handling other error
cases, so if we want to do this we'll probably need to test it out:

https://developer.gnome.org/glib/stable/glib-Key-value-file-parser.html

Otherwise the patch is fine:

Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>


> -	if (!igt_frame_dump_path)
> +	if (igt_key_file && !igt_frame_dump_path)
>		igt_frame_dump_path =
>			g_key_file_get_string(igt_key_file, "Common",
>					      "FrameDumpPath", &error);
>
>	g_clear_error(&error);
>
> -	ret = g_key_file_get_integer(igt_key_file, "DUT", "SuspendResumeDelay",
> -				     &error);
> +	if (igt_key_file)
> +		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);
> @@ -804,9 +803,10 @@ static void common_init_config(void)
>		if (igt_rc_device) {
>			igt_debug("Notice: using IGT_DEVICE env:\n");
>		} else {
> -			igt_rc_device =	g_key_file_get_string(igt_key_file,
> -							      "Common",
> -							      "Device", &error);
> +			if (igt_key_file)
> +				igt_rc_device =	g_key_file_get_string(igt_key_file,
> +								      "Common",
> +								      "Device", &error);
>			g_clear_error(&error);
>			if (igt_rc_device)
>				igt_debug("Notice: using .igtrc "
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev