From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by gabe.freedesktop.org (Postfix) with ESMTPS id AD62F6E4AD for ; Tue, 3 Nov 2020 05:13:27 +0000 (UTC) Date: Mon, 02 Nov 2020 21:13:25 -0800 Message-ID: <871rhbc9ai.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" In-Reply-To: <20201026084840.13061-1-petri.latvala@intel.com> References: <20201026084840.13061-1-petri.latvala@intel.com> MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Subject: Re: [igt-dev] [PATCH i-g-t] lib/igt_core: Don't return too early in common_init_config List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" To: Petri Latvala Cc: igt-dev@lists.freedesktop.org List-ID: 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 > Cc: Arkadiusz Hiler > --- > 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 > - 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