From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by gabe.freedesktop.org (Postfix) with ESMTPS id C376B6E90A for ; Tue, 3 Nov 2020 23:11:15 +0000 (UTC) Date: Tue, 03 Nov 2020 15:11:14 -0800 Message-ID: <87eelanii5.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" In-Reply-To: <20201103080513.GM7444@platvala-desk.ger.corp.intel.com> References: <20201026084840.13061-1-petri.latvala@intel.com> <871rhbc9ai.wl-ashutosh.dixit@intel.com> <20201103080513.GM7444@platvala-desk.ger.corp.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 Tue, 03 Nov 2020 00:05:13 -0800, Petri Latvala wrote: > > On Mon, Nov 02, 2020 at 09:13:25PM -0800, Dixit, Ashutosh wrote: > > 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: > > If it's not documented to work that way, I'm wary of calling those > functions with NULL. It could behave differently in different versions > of glib. OK, once again: Reviewed-by: Ashutosh Dixit > > > > 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