If your icon themes have circular inheritance, XcursorScanTheme recurses infinitely (until it eventually stack-overflows). We noticed this bug because the KDE mouse control panel dies when you try to launch it (if you have circular icon theme inheritence) -- the rest of KDE works fine. I hacked up a little patch that checks for circular Inherits. A quick overview of my patch: ~ renamed XcursorScanTheme to XcursorScanThemeAccum and added a third parameter, seen, which is a list of all the themes that have already been checked. ~ Added a new XcursorScanTheme that is a wrapper around XcursorScanThemeAccum so that the existing calls to XcursorScanTheme do not have to be changed. (To keep the patch simple). ~ Only recursively call XcursorScanThemeAccum if the inherited theme has not already been checked. ~ Note that I prepend new themes to the list, rather than append them, because it flows naturally with the recursion that way. This is the patch that we use internally, I am sure a few changes would be needed if it was applied to the official source. Please feel free to email if you have questions or flames. $ tla changes --diffs * looking for tos@lindows.com--2004/xcursor--circular-inherits--1.1.3--base-0 to compare with * comparing to tos@lindows.com--2004/xcursor--circular-inherits--1.1.3--base-0 M library.c M xcursorint.h * modified files --- orig/library.c +++ mod/library.c @@ -196,10 +196,26 @@ return result; } +int ListMem(StringList *seen, const char *theme) +{ + if (!seen) + return 0; + + while (seen->next != 0) { + printf("(%s,%s)\n",seen->theme,theme); + if (strcmp(seen->theme, theme) == 0) + return 1; + seen=seen->next; + } + if (strcmp(seen->theme, theme) == 0) + return 1; + return 0; +} + #define XCURSOR_SCAN_CORE ((FILE *) 1) static FILE * -XcursorScanTheme (const char *theme, const char *name) +XcursorScanThemeAccum (const char *theme, const char *name, StringList *seen) { FILE *f = 0; char *full; @@ -248,12 +264,28 @@ * Recurse to scan inherited themes */ for (i = inherits; i && f == 0; i = _XcursorNextPath (i)) - f = XcursorScanTheme (i, name); + { + if (!ListMem(seen, i)) + { + StringList *newSeen = (struct list_struct *)malloc(sizeof(StringList)); + newSeen->next = seen; + newSeen->theme = i; + f = XcursorScanThemeAccum (i, name, newSeen); + free(newSeen); + } + } + if (inherits) free (inherits); return f; } +static FILE * +XcursorScanTheme (const char *theme, const char *name) +{ + return (XcursorScanThemeAccum (theme, name, 0)); +} + XcursorImage * XcursorLibraryLoadImage (const char *file, const char *theme, int size) { --- orig/xcursorint.h +++ mod/xcursorint.h @@ -104,5 +104,12 @@ Cursor _XcursorCreateFontCursor (Display *dpy, unsigned int shape); + +typedef struct _StringList +{ + const char *theme; + struct _StringList *next; +} StringList; + #endif /* _XCURSORINT_H_ */
oops, I forgot the drop this debug line: + printf("(%s,%s)\n",seen->theme,theme);
Why not just a simple max link depth? That's a whole lot simpler and should be sufficient.
Short Answer: That works for me. Long Answer: The solution I presented is the simplest solution I could think of that could detect loops of arbitrary size. (It also roughly matches what KDE does). Unless, I misunderstand, the max depth solution will just guess that it is in a loop and bail out. I personally hate picking arbitrary limits, so I took the "easy" way out... On Linspire, we intentionally have the inheritance loop, clear-e <- crystalsvg <- hicolor <- clear-e <- ... If a user installs some other icon theme that inherits hicolor, then they would have the loop, new-theme <- hicolor <- clear-e <- crystalsvg <- hicolor <- ... So, as long as MAX_DEPTH is at least 4, that should work for us for now. The loop seems to recurse very quickly, so I think even a relatively large MAX_DEPTH will not have any significant performance impact...
Bugzilla Upgrade Mass Bug Change NEEDSINFO state was removed in Bugzilla 3.x, reopening any bugs previously listed as NEEDSINFO. - benjsc fd.o Wrangler
Is this still applicable? If so, let's fix this.
This is still an issue. I recently added a fallback to Adwaita for the default theme, same thing that Fedora has been doing for a while. What happens now is that once a cursor is missing in the Adwaita theme, libxcursor will recurse until death like described in the bugreport. This is reported by our Archlinux users at https://bugs.archlinux.org/task/40658
Diagnosed the problem a bit further, libxcursor will not loop when the default theme inherits some other theme. What goes into a loop is users who symlink /usr/share/icons/Adwaita to /usr/share/icons/default and a package manager that puts index.theme in there, causing Adwaita to inherit Adwaita. libxcursor could add guards against this, but it's a typical user configuration error.
Created attachment 129094 [details] [review] Fix for handling themes with circular dependency Attached is a recommendation for a patch which fixes this bug. I recently ran into this bug. I agree that this is caused by a configuration error by the user, however, this misconfiguration rendered me to be not be able to open any application which uses libXcursor - which are, as you can imagine, quite many. I think especially new users, who try to customize their desktop by some instructions found online, could be thrown-off by this bug, leaving them unable to use their system. And to be honest, which non tech-savvy user would think that her or his invalid cursor theme config would cause all applications to crash? Therefore I propose to include the attached patch or add a similar guard to avoid this.
Created attachment 129095 [details] [review] Updated patch with proper indentation.
Thanks, pushed to git master: To ssh://git.freedesktop.org/git/xorg/lib/libXcursor 4828abe..f64a8cc master -> master
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.