All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5] lib/igt_rc: Introduce generic config parser
@ 2026-08-13 16:25 Mark Yacoub
  0 siblings, 0 replies; only message in thread
From: Mark Yacoub @ 2026-08-13 16:25 UTC (permalink / raw)
  To: igt-dev; +Cc: kamil.konieczny, louis.chauvet, sebastian.brzezinka, Mark Yacoub

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="y", Size: 14907 bytes --]

Currently, libraries like unigraf and igt_core explicitly rely on GKeyFile
for reading configuration from .igtrc. Android builds do not natively supply
GLib, meaning these tools cannot be cleanly compiled.

This patch avoids platform-specific diverging implementations by dropping
the native GLib dependency from generic configuration access and parsing
`.igtrc` natively using a stripped-down, thread-safe `igt_list` implementation.

To prevent regressions for deeply-entrenched legacy Linux tools (such as Chamelium),
the native GLib fallback `GKeyFile` structures are retained and populated
parallel to the generic parser at startup.

v5:
 - Rebased onto upstream 'lib/igt_core: move igt_load_igtrc to igt_rc'
 - Safely run GLib loader parallel to generic parser at startup for Chamelium

v4:
 - Implement igt_rc_get_double() to parse floating point configs natively.
 - Migrate igt_core.c over to igt_rc_get_double() to drop GKeyFile dependencies.
 - Add robust standard bounds checking (ERANGE and unmatched digits) to number parsers.

v3:
 - Drop Android-specific fallback paths; use IGT_CONFIG_PATH natively.
 - Drop glib Linux wrappers entirely; unify a single generic parser for all platforms.
 - Port manual linked-list tracking over to standard igt_list.h APIs.
 - Inherit base 0 for strtol to safely parse hex masks.
 - Use robust PATH_MAX for dynamic config paths.
 - Update pointer assignments, array indexing [0], and public docstrings.

v2:
 - Drop the GKeyFile abstraction fakes from android/glib.h.
 - Introduce a generic wrapper (igt_rc.h) instead of modifying glib.h.
---
 lib/igt_core.c               |  37 ++----
 lib/igt_rc.c                 | 248 ++++++++++++++++++++++++++++++++++-
 lib/igt_rc.h                 |   6 +
 lib/vendor/unigraf/unigraf.c |  92 ++++++-------
 4 files changed, 297 insertions(+), 86 deletions(-)

diff --git a/lib/igt_core.c b/lib/igt_core.c
index 2f737b01a..b3efab247 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -974,39 +974,24 @@ static void oom_adjust_for_doom(void)
 
 static void common_init_config(void)
 {
-	GError *error = NULL;
 	int ret = 0;
 	static double timeout = 0.0;
 
-	igt_key_file = igt_load_igtrc();
-
-	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);
-
-	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);
+	igt_key_file = igt_load_igtrc();
+	if (!igt_frame_dump_path)
+		igt_frame_dump_path = igt_rc_get_string("Common", "FrameDumpPath");
 
-	if (ret != 0)
-		igt_set_autoresume_delay(ret);
+	if (igt_rc_get_integer("DUT", "SuspendResumeDelay", &ret)) {
+		if (ret != 0)
+			igt_set_autoresume_delay(ret);
+	}
 
-	if (igt_key_file)
-		timeout = g_key_file_get_double(igt_key_file, "DUT", "DisplayDetectTimeout",
-						&error);
-	if (error) {
+	if (!igt_rc_get_double("DUT", "DisplayDetectTimeout", &timeout)) {
 		igt_debug("Failed to read DisplayDetectTimeout, defaulting to %f\n",
 			  DEFAULT_DETECT_TIMEOUT);
-		g_clear_error(&error);
 		timeout = DEFAULT_DETECT_TIMEOUT;
 	}
-	g_clear_error(&error);
 	igt_set_default_display_detect_timeout(timeout);
 
 	/* Adding filters, order .igtrc, IGT_DEVICE, --device filter */
@@ -1016,11 +1001,7 @@ static void common_init_config(void)
 		if (igt_rc_device) {
 			igt_debug("Notice: using IGT_DEVICE env:\n");
 		} else {
-			if (igt_key_file)
-				igt_rc_device =	g_key_file_get_string(igt_key_file,
-								      "Common",
-								      "Device", &error);
-			g_clear_error(&error);
+			igt_rc_device = igt_rc_get_string("Common", "Device");
 			if (igt_rc_device)
 				igt_debug("Notice: using .igtrc "
 					  "Common::Device:\n");
diff --git a/lib/igt_rc.c b/lib/igt_rc.c
index b7f1731fe..b5035d788 100644
--- a/lib/igt_rc.c
+++ b/lib/igt_rc.c
@@ -3,16 +3,252 @@
  * Copyright © 2026 Intel Corporation
  */
 
+#include "igt_rc.h"
+
 #include <stdio.h>
 #include <stdlib.h>
+#include <errno.h>
+#include <locale.h>
+#include <string.h>
+#include <strings.h>
+#include <ctype.h>
+#include <pthread.h>
+#include <limits.h>
+#include "igt_list.h"
 
-#ifndef ANDROID
-#include <glib.h>
-#else
-#include "android/glib.h"
-#endif
+struct igt_key_entry {
+	char *group;
+	char *key;
+	char *value;
+	struct igt_list_head link;
+};
 
-#include "igt_rc.h"
+static IGT_LIST_HEAD(rc_entries);
+static pthread_once_t rc_once_control = PTHREAD_ONCE_INIT;
+
+static char *trim_whitespace(char *str)
+{
+	char *end;
+
+	while (isspace((unsigned char)str[0]))
+		str++;
+
+	if (str[0] == 0)
+		return str;
+
+	end = str + strlen(str) - 1;
+	while (end > str && isspace((unsigned char)end[0]))
+		end--;
+
+	end[1] = '\0';
+	return str;
+}
+
+static void load_igtrc_once(void)
+{
+	FILE *fp;
+	char *line = NULL;
+	size_t len = 0;
+	ssize_t read;
+	char *current_group = NULL;
+	char path[PATH_MAX];
+
+	char *config_path = getenv("IGT_CONFIG_PATH");
+	if (config_path) {
+		snprintf(path, sizeof(path), "%s", config_path);
+	} else {
+		char *home = getenv("HOME");
+		if (!home)
+			home = "";
+		snprintf(path, sizeof(path), "%s/.igtrc", home);
+	}
+
+	fp = fopen(path, "r");
+	if (!fp)
+		return;
+
+	while ((read = getline(&line, &len, fp)) != -1) {
+		char *trimmed = trim_whitespace(line);
+
+		if (trimmed[0] == '\0' || trimmed[0] == '#' || trimmed[0] == ';')
+			continue;
+
+		if (trimmed[0] == '[' && trimmed[strlen(trimmed) - 1] == ']') {
+			free(current_group);
+			trimmed[strlen(trimmed) - 1] = '\0';
+			current_group = strdup(trimmed + 1);
+			continue;
+		}
+
+		if (current_group) {
+			char *eq = strchr(trimmed, '=');
+
+			if (eq) {
+				char *key;
+				char *value;
+				struct igt_key_entry *entry;
+
+				eq[0] = '\0';
+				value = eq + 1;
+				key = trim_whitespace(trimmed);
+				value = trim_whitespace(value);
+
+				entry = calloc(1, sizeof(*entry));
+				entry->group = strdup(current_group);
+				entry->key = strdup(key);
+				entry->value = strdup(value);
+				igt_list_add_tail(&entry->link, &rc_entries);
+			}
+		}
+	}
+
+	free(current_group);
+	free(line);
+	fclose(fp);
+}
+
+__attribute__((destructor))
+static void free_igtrc(void)
+{
+	struct igt_key_entry *curr, *tmp;
+
+	igt_list_for_each_entry_safe(curr, tmp, &rc_entries, link) {
+		free(curr->group);
+		free(curr->key);
+		free(curr->value);
+		free(curr);
+	}
+	IGT_INIT_LIST_HEAD(&rc_entries);
+}
+
+/**
+ * igt_rc_get_string:
+ * @group_name: The group name in the config file.
+ * @key: The key to look up.
+ *
+ * Looks up a string configuration value in the `.igtrc` file.
+ * The returned string is newly allocated and must be freed by 
+ * the caller using free().
+ *
+ * Returns: A newly allocated string containing the value, or NULL if not found.
+ */
+char *igt_rc_get_string(const char *group_name, const char *key)
+{
+	char *last_match = NULL;
+	struct igt_key_entry *curr;
+
+	pthread_once(&rc_once_control, load_igtrc_once);
+
+	igt_list_for_each_entry(curr, &rc_entries, link) {
+		if (strcmp(curr->group, group_name) == 0 && strcmp(curr->key, key) == 0)
+			last_match = curr->value;
+	}
+
+	return last_match ? strdup(last_match) : NULL;
+}
+
+/**
+ * igt_rc_get_boolean:
+ * @group_name: The group name in the config file.
+ * @key: The key to look up.
+ * @out: Pointer to a boolean where the result will be stored.
+ *
+ * Looks up a boolean configuration value in the `.igtrc` file.
+ * Parses standard boolean representations like "true", "false", "1", "0".
+ *
+ * Returns: true if the key exists and was successfully parsed, false otherwise.
+ */
+bool igt_rc_get_boolean(const char *group_name, const char *key, bool *out)
+{
+	char *val = igt_rc_get_string(group_name, key);
+
+	if (!val)
+		return false;
+
+	if (strcasecmp(val, "true") == 0 || strcmp(val, "1") == 0) {
+		*out = true;
+	} else if (strcasecmp(val, "false") == 0 || strcmp(val, "0") == 0) {
+		*out = false;
+	} else {
+		free(val);
+		return false;
+	}
+
+	free(val);
+	return true;
+}
+
+/**
+ * igt_rc_get_integer:
+ * @group_name: The group name in the config file.
+ * @key: The key to look up.
+ * @out: Pointer to an integer where the result will be stored.
+ *
+ * Looks up an integer configuration value in the `.igtrc` file.
+ *
+ * Returns: true if the key exists and was successfully parsed, false otherwise.
+ */
+bool igt_rc_get_integer(const char *group_name, const char *key, int *out)
+{
+	char *val = igt_rc_get_string(group_name, key);
+	char *endptr;
+	long lval;
+
+	if (!val)
+		return false;
+
+	errno = 0;
+	lval = strtol(val, &endptr, 0);
+	if (endptr == val || endptr[0] != '\0' || errno == ERANGE) {
+		free(val);
+		return false;
+	}
+
+	*out = (int)lval;
+	free(val);
+	return true;
+}
+
+/**
+ * igt_rc_get_double:
+ * @group_name: The group name in the config file.
+ * @key: The key to look up.
+ * @out: Pointer to a double where the result will be stored.
+ *
+ * Looks up a double configuration value in the `.igtrc` file.
+ *
+ * Returns: true if the key exists and was successfully parsed, false otherwise.
+ */
+bool igt_rc_get_double(const char *group_name, const char *key, double *out)
+{
+	char *val = igt_rc_get_string(group_name, key);
+	char *endptr;
+	double dval;
+
+	char *dot;
+	struct lconv *lc;
+
+	if (!val)
+		return false;
+
+	/* Handle locale-specific decimal separator for strtod */
+	dot = strchr(val, '.');
+	lc = localeconv();
+	if (dot && lc && lc->decimal_point && lc->decimal_point[0] != '.') {
+		*dot = lc->decimal_point[0];
+	}
+
+	errno = 0;
+	dval = strtod(val, &endptr);
+	if (endptr == val || endptr[0] != '\0' || errno == ERANGE) {
+		free(val);
+		return false;
+	}
+
+	*out = dval;
+	free(val);
+	return true;
+}
 
 /**
  * igt_load_igtrc:
diff --git a/lib/igt_rc.h b/lib/igt_rc.h
index 3b7179808..1fa4f9a11 100644
--- a/lib/igt_rc.h
+++ b/lib/igt_rc.h
@@ -30,9 +30,15 @@
 #else
 #include "android/glib.h"
 #endif
+#include <stdbool.h>
 
 extern GKeyFile *igt_key_file;
 
 struct _GKeyFile *igt_load_igtrc(void);
 
+char *igt_rc_get_string(const char *group_name, const char *key);
+bool igt_rc_get_boolean(const char *group_name, const char *key, bool *out);
+bool igt_rc_get_integer(const char *group_name, const char *key, int *out);
+bool igt_rc_get_double(const char *group_name, const char *key, double *out);
+
 #endif /* IGT_RC_H */
diff --git a/lib/vendor/unigraf/unigraf.c b/lib/vendor/unigraf/unigraf.c
index 30ee3c72b..a6b3008eb 100644
--- a/lib/vendor/unigraf/unigraf.c
+++ b/lib/vendor/unigraf/unigraf.c
@@ -364,7 +364,6 @@ int unigraf_get_connector_id_by_stream(int drm_fd, int stream_id)
 bool unigraf_open_device(int drm_fd)
 {
 	TSI_RESULT r;
-	GError *cfg_error = NULL;
 	char *cfg_device = NULL;
 	char *cfg_role = NULL;
 	char *cfg_input = NULL;
@@ -382,63 +381,52 @@ bool unigraf_open_device(int drm_fd)
 
 	unigraf_init();
 
-	if (igt_key_file) {
-		cfg_device = g_key_file_get_string(igt_key_file, UNIGRAF_CONFIG_GROUP,
-						   UNIGRAF_CONFIG_DEVICE_NAME, &cfg_error);
-		if (cfg_error) {
-			unigraf_debug("No device name configured, uses first device available.\n");
-			cfg_device = NULL;
-		}
+	cfg_device = igt_rc_get_string(UNIGRAF_CONFIG_GROUP,
+				       UNIGRAF_CONFIG_DEVICE_NAME);
+	if (!cfg_device) {
+		unigraf_debug("No device name configured, uses first device available.\n");
+		cfg_device = NULL;
+	}
 
-		cfg_error = NULL;
-		cfg_role = g_key_file_get_string(igt_key_file, UNIGRAF_CONFIG_GROUP,
-						 UNIGRAF_CONFIG_DEVICE_ROLE, &cfg_error);
-		if (cfg_error) {
-			unigraf_debug("No device role configured.\n");
-			cfg_role = NULL;
-		}
+	cfg_role = igt_rc_get_string(UNIGRAF_CONFIG_GROUP,
+				     UNIGRAF_CONFIG_DEVICE_ROLE);
+	if (!cfg_role) {
+		unigraf_debug("No device role configured.\n");
+		cfg_role = NULL;
+	}
 
-		cfg_error = NULL;
-		cfg_input = g_key_file_get_string(igt_key_file, UNIGRAF_CONFIG_GROUP,
-						  UNIGRAF_CONFIG_INPUT_NAME, &cfg_error);
-		if (cfg_error) {
-			unigraf_debug("No input name configured.\n");
-			cfg_input = NULL;
-		}
+	cfg_input = igt_rc_get_string(UNIGRAF_CONFIG_GROUP,
+				      UNIGRAF_CONFIG_INPUT_NAME);
+	if (!cfg_input) {
+		unigraf_debug("No input name configured.\n");
+		cfg_input = NULL;
+	}
 
-		cfg_error = NULL;
-		unigraf_connector_name = g_key_file_get_string(igt_key_file, UNIGRAF_CONFIG_GROUP,
-							       UNIGRAF_CONFIG_CONNECTOR_NAME,
-							       &cfg_error);
-		if (cfg_error) {
-			unigraf_debug("No connector name configured, will autodetect.\n");
-			unigraf_connector_name = NULL;
-		}
+	unigraf_connector_name = igt_rc_get_string(UNIGRAF_CONFIG_GROUP,
+						   UNIGRAF_CONFIG_CONNECTOR_NAME);
+	if (!unigraf_connector_name) {
+		unigraf_debug("No connector name configured, will autodetect.\n");
+		unigraf_connector_name = NULL;
+	}
 
-		cfg_error = NULL;
-		cfg_edid_name = g_key_file_get_string(igt_key_file, UNIGRAF_CONFIG_GROUP,
-						      UNIGRAF_CONFIG_EDID_NAME, &cfg_error);
-		if (cfg_error) {
-			unigraf_debug("No default EDID set, use IGT default.\n");
-			cfg_edid_name = NULL;
-		}
+	cfg_edid_name = igt_rc_get_string(UNIGRAF_CONFIG_GROUP,
+					  UNIGRAF_CONFIG_EDID_NAME);
+	if (!cfg_edid_name) {
+		unigraf_debug("No default EDID set, using IGT default.\n");
+		cfg_edid_name = NULL;
+	}
 
-		cfg_error = NULL;
-		unigraf_crc = g_key_file_get_boolean(igt_key_file, UNIGRAF_CONFIG_GROUP,
-						     UNIGRAF_CONFIG_USE_CRC_NAME, &cfg_error);
-		if (cfg_error) {
-			unigraf_debug("CRC usage not configured, using unigraf CRC.\n");
-			unigraf_crc = true;
-		}
+	if (!igt_rc_get_boolean(UNIGRAF_CONFIG_GROUP,
+				UNIGRAF_CONFIG_USE_CRC_NAME, &unigraf_crc)) {
+		unigraf_debug("CRC usage not configured, using unigraf CRC.\n");
+		unigraf_crc = true;
+	}
 
-		cfg_error = NULL;
-		unigraf_stream_count = g_key_file_get_integer(igt_key_file, UNIGRAF_CONFIG_GROUP,
-							      UNIGRAF_CONFIG_MST_STREAM_COUNT,
-							      &cfg_error);
-		if (cfg_error) {
-			unigraf_debug("MST usage not configured, using SST.\n");
-			unigraf_stream_count = 0;
-		}
+	if (!igt_rc_get_integer(UNIGRAF_CONFIG_GROUP,
+				UNIGRAF_CONFIG_MST_STREAM_COUNT,
+				&unigraf_stream_count)) {
+		unigraf_debug("MST usage not configured, using SST.\n");
+		unigraf_stream_count = 0;
 	}
 
 	unigraf_assert(TSIX_DEV_RescanDevices(0, TSI_DEVCAP_VIDEO_CAPTURE, 0));
-- 
2.55.0.691.gc56d675ccc-goog


^ permalink raw reply related	[flat|nested] only message in thread

only message in thread, other threads:[~2026-08-13 16:26 UTC | newest]

Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-13 16:25 [PATCH v5] lib/igt_rc: Introduce generic config parser Mark Yacoub

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.