From b47ca9858ee11bdbd08f1e31102116ff51cb6be5 Mon Sep 17 00:00:00 2001
From: Tobias Stoeckmann <tobias@stoeckmann.org>
Date: Sun, 2 May 2021 16:00:25 +0200
Subject: Handle more theme loop situations

This is a follow up for commit f64a8cc1a65dcad4294e2988b402a34175019663
resulting from https://bugs.freedesktop.org/show_bug.cgi?id=3603

The current loop detection only works for direct self references but not
for transitive ones. Limiting the inheritance depth fixes this issue as
suggested by Keith Packard.

I avoided the introduction of a recursion function. Instead I modified
XcursorScanTheme to work iterative.

The current recursion code adds the "Inherits=..." line to heap and has
an iteration variable to go through all themes listed in that line per
recursion. This is covered with the newly introduced XcursorInherit
struct with its fields "line" and "theme". Since "theme" points into
"line", only "line" has to be freed eventually.

If a fixed inheritage limit of 32 is reached, the code stops processing
and returns NULL. It also returns NULL if it detects the initial theme
in one of the inheritages to break the loop early on.

Last but not least I removed the printf statement. The only situation in
which libXcursor writes to stdout is when it is explicitly requested.

Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
---
 src/library.c | 109 +++++++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 73 insertions(+), 36 deletions(-)

(limited to 'src')

diff --git a/src/library.c b/src/library.c
index bf637ea..238f8ac 100644
--- a/src/library.c
+++ b/src/library.c
@@ -32,6 +32,11 @@
 #define XCURSORPATH "~/.local/share/icons:~/.icons:/usr/share/icons:/usr/share/pixmaps:"ICONDIR
 #endif
 
+typedef struct XcursorInherit {
+    char	*line;
+    const char	*theme;
+} XcursorInherit;
+
 const char *
 XcursorLibraryPath (void)
 {
@@ -206,16 +211,17 @@ _XcursorThemeInherits (const char *full)
 }
 
 #define XCURSOR_SCAN_CORE   ((FILE *) 1)
+#define MAX_INHERITS_DEPTH  32
 
 static FILE *
 XcursorScanTheme (const char *theme, const char *name)
 {
-    FILE	*f = NULL;
-    char	*full;
-    char	*dir;
-    const char  *path;
-    char	*inherits = NULL;
-    const char	*i;
+    FILE		*f = NULL;
+    char		*full;
+    char		*dir;
+    const char		*path;
+    XcursorInherit	 inherits[MAX_INHERITS_DEPTH + 1];
+    int			 d;
 
     if (!theme || !name)
         return NULL;
@@ -228,46 +234,77 @@ XcursorScanTheme (const char *theme, const char *name)
      */
     if (!strcmp (theme, XCURSOR_CORE_THEME) && XcursorLibraryShape (name) >= 0)
 	return XCURSOR_SCAN_CORE;
-    /*
-     * Scan this theme
-     */
-    for (path = XcursorLibraryPath ();
-	 path && f == NULL;
-	 path = _XcursorNextPath (path))
+
+    memset (inherits, 0, sizeof (inherits));
+
+    d = 0;
+    inherits[d].theme = theme;
+
+    while (f == NULL && d >= 0 && inherits[d].theme != NULL)
     {
-	dir = _XcursorBuildThemeDir (path, theme);
-	if (dir)
+	/*
+	 * Scan this theme
+	 */
+	for (path = XcursorLibraryPath ();
+	     path && f == NULL;
+	     path = _XcursorNextPath (path))
 	{
-	    full = _XcursorBuildFullname (dir, "cursors", name);
-	    if (full)
-	    {
-		f = fopen (full, "r");
-		free (full);
-	    }
-	    if (!f && !inherits)
+	    dir = _XcursorBuildThemeDir (path, inherits[d].theme);
+	    if (dir)
 	    {
-		full = _XcursorBuildFullname (dir, "", "index.theme");
+		full = _XcursorBuildFullname (dir, "cursors", name);
 		if (full)
 		{
-		    inherits = _XcursorThemeInherits (full);
+		    f = fopen (full, "r");
 		    free (full);
 		}
+		if (!f && inherits[d + 1].line == NULL)
+		{
+		    if (d + 1 >= MAX_INHERITS_DEPTH)
+		    {
+			free (dir);
+			goto cleanup;
+		    }
+		    full = _XcursorBuildFullname (dir, "", "index.theme");
+		    if (full)
+		    {
+			inherits[d + 1].line = _XcursorThemeInherits (full);
+			inherits[d + 1].theme = inherits[d + 1].line;
+			free (full);
+		    }
+		}
+		free (dir);
 	    }
-	    free (dir);
 	}
+
+	if (inherits[d + 1].line == NULL)
+	{
+	    if (d == 0)
+		inherits[d].theme = NULL;
+	    else
+	    {
+		inherits[d].theme = _XcursorNextPath (inherits[d].theme);
+		if (inherits[d].theme == NULL)
+		{
+		    free (inherits[d].line);
+		    inherits[d--].line = NULL;
+		}
+	    }
+	}
+	else
+	    d++;
+
+	/*
+	 * Detect and break self reference loop early on.
+	 */
+	if (inherits[d].theme != NULL && strcmp (inherits[d].theme, theme) == 0)
+	    break;
     }
-    /*
-     * Recurse to scan inherited themes
-     */
-    for (i = inherits; i && f == NULL; i = _XcursorNextPath (i))
-    {
-        if (strcmp(i, theme) != 0)
-            f = XcursorScanTheme (i, name);
-        else
-            printf("Not calling XcursorScanTheme because of circular dependency: %s. %s", i, name);
-    }
-    if (inherits != NULL)
-	free (inherits);
+
+cleanup:
+    for (d = 1; d <= MAX_INHERITS_DEPTH; d++)
+	free (inherits[d].line);
+
     return f;
 }
 
-- 
cgit v1.2.1