[igt-dev] [PATCH i-g-t 1/4] lib: Export a function for loading igtrc

[Arkadiusz Hiler]

On Fri, Sep 13, 2019 at 03:48:42PM +0300, Petri Latvala wrote:
> On Fri, Sep 13, 2019 at 01:50:10PM +0300, Arkadiusz Hiler wrote:
> > On Fri, Sep 13, 2019 at 12:26:21PM +0300, Petri Latvala wrote:
> > > On Tue, Sep 10, 2019 at 03:41:24PM +0300, Arkadiusz Hiler wrote:
> > > > On Mon, Sep 09, 2019 at 02:38:06PM +0300, Petri Latvala wrote:
> > > > > Signed-off-by: Petri Latvala <petri.latvala at intel.com>
> > > > > Cc: Arkadiusz Hiler <arkadiusz.hiler at intel.com>
> > > > > ---
> > > > >  lib/igt_core.c | 40 +++++++++++++++++++++++++++++++---------
> > > > >  lib/igt_core.h |  2 ++
> > > > >  2 files changed, 33 insertions(+), 9 deletions(-)
> > > > > 
> > > > > diff --git a/lib/igt_core.c b/lib/igt_core.c
> > > > > index 1cbb09f9..940913c1 100644
> > > > > --- a/lib/igt_core.c
> > > > > +++ b/lib/igt_core.c
> > > > > @@ -642,11 +642,21 @@ static void oom_adjust_for_doom(void)
> > > > >  
> > > > >  }
> > > > >  
> > > > > -static void common_init_config(void)
> > > > > +/**
> > > > > + * load_igtrc:
> > > > > + *
> > > > > + * Load .igtrc from the path pointed to by #IGT_CONFIG_PATH or from
> > > > > + * home directory if that is not set. The returned keyfile needs to be
> > > > > + * deallocated using g_key_file_free().
> > > > > + *
> > > > > + * Returns: Pointer to the keyfile, NULL on error.
> > > > > + */
> > > > > +GKeyFile *igt_load_igtrc(void)
> > > > >  {
> > > > >  	char *key_file_env = NULL;
> > > > >  	char *key_file_loc = NULL;
> > > > >  	GError *error = NULL;
> > > > > +	GKeyFile *file;
> > > > >  	int ret;
> > > > >  
> > > > >  	/* Determine igt config path */
> > > > > @@ -659,19 +669,35 @@ static void common_init_config(void)
> > > > >  	}
> > > > >  
> > > > >  	/* Load igt config file */
> > > > > -	igt_key_file = g_key_file_new();
> > > > > -	ret = g_key_file_load_from_file(igt_key_file, key_file_loc,
> > > > > +	file = g_key_file_new();
> > > > > +	ret = g_key_file_load_from_file(file, key_file_loc,
> > > > >  					G_KEY_FILE_NONE, &error);
> > > > >  	if (!ret) {
> > > > >  		g_error_free(error);
> > > > > -		g_key_file_free(igt_key_file);
> > > > > -		igt_key_file = NULL;
> > > > > +		g_key_file_free(file);
> > > > > +		file = NULL;
> > > > >  
> > > > >  		goto out;
> > > > >  	}
> > > > >  
> > > > >  	g_clear_error(&error);
> > > > >  
> > > > > + out:
> > > > > +	if (!key_file_env && key_file_loc)
> > > > > +		free(key_file_loc);
> > > > > +
> > > > > +	return file;
> > > > > +}
> > > > > +
> > > > > +static void common_init_config(void)
> > > > > +{
> > > > > +	GError *error = NULL;
> > > > > +	int ret;
> > > > > +
> > > > > +	igt_key_file = igt_load_igtrc();
> > > > > +	if (!igt_key_file)
> > > > > +		return;
> > > > > +
> > > > >  	if (!igt_frame_dump_path)
> > > > >  		igt_frame_dump_path =
> > > > >  			g_key_file_get_string(igt_key_file, "Common",
> > > > > @@ -687,10 +713,6 @@ static void common_init_config(void)
> > > > >  
> > > > >  	if (ret != 0)
> > > > >  		igt_set_autoresume_delay(ret);
> > > > > -
> > > > > -out:
> > > > > -	if (!key_file_env && key_file_loc)
> > > > > -		free(key_file_loc);
> > > > >  }
> > > > >  
> > > > >  static void common_init_env(void)
> > > > > diff --git a/lib/igt_core.h b/lib/igt_core.h
> > > > > index 177d2431..521cda10 100644
> > > > > --- a/lib/igt_core.h
> > > > > +++ b/lib/igt_core.h
> > > > > @@ -83,6 +83,8 @@ extern const char* __igt_test_description __attribute__((weak));
> > > > >  extern bool __igt_plain_output;
> > > > >  extern char *igt_frame_dump_path;
> > > > >  
> > > > > +struct _GKeyFile *igt_load_igtrc(void);
> > > > 
> > > > Why not GKeyFile *...?
> > > 
> > > 
> > > That would require #include <glib.h> in igt_core.h and spread that
> > > dependency further than feels ok.
> > 
> > Oh. Oh.... Then where do we get struct _GKeyFile from?
> 
> Nowhere, it being a complete type is not needed to have a valid
> pointer type.

In the rush I haven't noticed the lack of struct, and yeah typedes do
not behave the same as opawue struct. Sorry for the dumb question.

> (For the record, "struct GKeyFile *" is wrong, because when we
> actually have glib.h, there won't be a "struct GKeyFile", there's a
> "struct _GKeyFile" and a typedef of that into "GKeyFile")

bit ugly, bit haxy, but I don't see a better way

Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler at intel.com>