Hi, with CVs from today (2005-01-11), I''m able to trigger an easy crash with fc-cat. To reproduce : make sure you have an empty sub-directory in your fonts path rm -rf /var/cache/fontconfig/* xterm -fa sans fc-cat ~/.fonts.cache-2 fc-cat will crash with the following stacktrace, when trying to print info for the empty directory : fc-cat: printing global cache contents for dir /usr/X11R6/lib/X11/fonts/mdk Program received signal SIGSEGV, Segmentation fault. 0xb7ed1a6b in FcConfigAddFontDir (config=0x0, d=0x0) at fccfg.c:388 388 return FcStrSetAddFilename (config->fontDirs, d); (gdb) bt #0 0xb7ed1a6b in FcConfigAddFontDir (config=0x0, d=0x0) at fccfg.c:388 #1 0x0804a19c in FcDirCacheConsume (fd=7, dir=0xbf82dae7 "/usr/X11R6/lib/X11/fonts/mdk", set=0x8051008, config=0x0) at ../src/fccache.c:972 #2 0x0804bbea in main (argc=2, argv=0xbf830bf4) at fc-cat.c:205 #3 0xb7bcbe40 in __libc_start_main () from /lib/tls/libc.so.6 #4 0x08049a61 in _start () at ../sysdeps/i386/elf/start.S:119 I''m not sure how to fix this cleanly : either handle null config in FcConfigAddFontDir nicely or prevent the call in FcDirCacheConsume. -- Frederic Crozat <fcrozat@mandriva.com> Mandriva
Frederic Crozat wrote:> Hi, > > with CVs from today (2005-01-11), I''m able to trigger an easy crash with > fc-cat. To reproduce : > make sure you have an empty sub-directory in your fonts path > rm -rf /var/cache/fontconfig/* > xterm -fa sans > fc-cat ~/.fonts.cache-2This is fixed now. pat
Patrick Lam <plam@MIT.EDU> ????????:> Frederic Crozat wrote: >> Hi, >> >> with CVs from today (2005-01-11), I''m able to trigger an easy crash with >> fc-cat. To reproduce : >> make sure you have an empty sub-directory in your fonts path >> rm -rf /var/cache/fontconfig/* >> xterm -fa sans >> fc-cat ~/.fonts.cache-2 > > This is fixed now.Yes, this seems to be really fixed in the CVS checkout from yesterday. -- Mike FABIAN <mfabian@suse.de> http://www.suse.de/~mfabian ?????????????
Mike FABIAN
2006-Jan-17 06:43 UTC
[Fontconfig] Yet another easy to reproduce crash in fontconfig
mfabian@magellan:~$ sudo rm /var/cache/fontconfig/*cache* mfabian@magellan:~$ rm .fonts.cache-2 mfabian@magellan:~$ fc-match sans arial.ttf: "Arial" "Regular" mfabian@magellan:~$ touch .fonts.conf mfabian@magellan:~$ fc-match sans ????????????? (core dumped) mfabian@magellan:~$ (gdb) bt #0 0x00002aaaab12c420 in strlen () from /lib64/libc.so.6 #1 0x00002aaaaabd7075 in FcGlobalCacheReadDir () from /usr/lib64/libfontconfig.so.1 #2 0x00002aaaaabde7c3 in FcDirScanConfig () from /usr/lib64/libfontconfig.so.1 #3 0x00002aaaaabd7b56 in FcCacheReadDirs () from /usr/lib64/libfontconfig.so.1 #4 0x00002aaaaabd7b87 in FcCacheReadDirs () from /usr/lib64/libfontconfig.so.1 #5 0x00002aaaaabd7cbf in FcCacheRead () from /usr/lib64/libfontconfig.so.1 #6 0x00002aaaaabd9bad in FcConfigBuildFonts () from /usr/lib64/libfontconfig.so.1 #7 0x00002aaaaabe1bfb in FcInitLoadConfigAndFonts () from /usr/lib64/libfontconfig.so.1 #8 0x00002aaaaabe1cb5 in FcInit () from /usr/lib64/libfontconfig.so.1 #9 0x0000000000400e6b in main () (gdb) -- Mike FABIAN <mfabian@suse.de> http://www.suse.de/~mfabian ?????????????
Mike FABIAN
2006-Jan-18 07:30 UTC
[Fontconfig] Re: Yet another easy to reproduce crash in fontconfig
Mike FABIAN <mfabian@suse.de> ????????:> mfabian@magellan:~$ sudo rm /var/cache/fontconfig/*cache* > mfabian@magellan:~$ rm .fonts.cache-2 > mfabian@magellan:~$ fc-match sans > arial.ttf: "Arial" "Regular" > mfabian@magellan:~$ touch .fonts.conf > mfabian@magellan:~$ fc-match sans > ????????????? (core dumped) > mfabian@magellan:~$ > > (gdb) bt > #0 0x00002aaaab12c420 in strlen () from /lib64/libc.so.6 > #1 0x00002aaaaabd7075 in FcGlobalCacheReadDir () from /usr/lib64/libfontconfig.so.1The attached patch seems to fix the problem for me. Without that patch, directories which are subdirectories of directories listed in the config files (/etc/fonts/fonts.conf etc) were added too late to the list of font directories in "FcConfig *config". For example, when FcDirScanConfig() was called with the argument "/usr/share/fonts/bdf" the list of font directories in config->fontDirs->strs still contained only the higher level directory "/usr/share/fonts" because only that was listed in the config files, not the "bdf" subdirectory. FcDirScanConfig() then calls FcGlobalCacheReadDir() before adding the subdirectory to the list, FcGlobalCacheReadDir() calls FcConfigNormalizeFontDir(dir) which returns 0 because the directory is not yet in the list and then it crashes in strlen(dir) of course. To fix this, I moved the call to FcConfigAddFontDir (config, dir) upwards in FcDirScanConfig() to add the directory *before* FcGlobalCacheReadDir() is called. -------------- next part -------------- A non-text attachment was scrubbed... Name: add-dir-early-in-FcDirScanConfig.patch Type: text/x-patch Size: 568 bytes Desc: not available Url : http://lists.freedesktop.org/archives/fontconfig/attachments/20060118/daad7837/add-dir-early-in-FcDirScanConfig.bin -------------- next part -------------- -- Mike FABIAN <mfabian@suse.de> http://www.suse.de/~mfabian ?????????????
Patrick Lam
2006-Jan-18 07:46 UTC
[Fontconfig] Re: Yet another easy to reproduce crash in fontconfig
Mike FABIAN wrote:> The attached patch seems to fix the problem for me.Excellent, this was just about the sort of patch I was looking for. I''ve committed it. pat
Patch by Andreas Schwab <schwab@suse.de>. See also: http://bugzilla.novell.com/show_bug.cgi?id=142215 -------------- next part -------------- A non-text attachment was scrubbed... Name: unaligned-access.patch Type: text/x-patch Size: 1669 bytes Desc: not available Url : http://lists.freedesktop.org/archives/fontconfig/attachments/20060119/4fc0b304/unaligned-access.bin -------------- next part -------------- -- Mike FABIAN <mfabian@suse.de> http://www.suse.de/~mfabian ?????????????
Mike FABIAN wrote:> Patch by Andreas Schwab <schwab@suse.de>. > > See also: > > http://bugzilla.novell.com/show_bug.cgi?id=142215Looks good, I''ve committed it. Are there any of the /var/cache/fontconfig bugs still outstanding? I''ll try to take a look at any leftover ones there sooner or later, if folks let me know what''s up... pat
Patrick Lam <plam@MIT.EDU> ????????:> Mike FABIAN wrote: >> Patch by Andreas Schwab <schwab@suse.de>. >> >> See also: >> >> http://bugzilla.novell.com/show_bug.cgi?id=142215 > > Looks good, I''ve committed it. > > Are there any of the /var/cache/fontconfig bugs still outstanding? I''ll > try to take a look at any leftover ones there sooner or later, if folks > let me know what''s up...I think there are still problems with the normalizing of the font directory names. I posted a patch to this list recently to fix the problem that fc-cache /usr/share/fonts fc-cache /usr/share/fonts/ cd /usr/share/fonts; fc-cache . generate different cache files, but this patch had the problem that it neede to call FcInitLoadConfigAndFonts () instead of FcInitLoadConfig () to be able to use FcConfigNormalizeFontDir(). But this has the severe disadvantage that FcInitLoadConfigAndFonts () is very expensive, therefore this is not a good solution. Frederic Crozat''s approach using realpath () worked much better. But you said realpath () cannot be used because it appears to be problematic and not portable according to its man-page. Yesterday R?diger Oertel <ro@suse.de> had the idea to use the inodes and device ids of the directory to cache to create the md5sums for the file names of the cache files and Dirk Mueller <dmueller@suse.de> made a patch to try this approach. This idea seems to have the problem that for some file systems inodes may change when mounting the same file system again. For example with a vfat file system: root@magellan:~# mount | grep mnt /tmp/image on /mnt type vfat (rw,loop=/dev/loop0) root@magellan:~# ls -i /mnt 1157 a* 1158 b* 1159 c* 1160 d* 1161 e* 1162 f* 1163 g* 1164 h* root@magellan:~# umount /mnt root@magellan:~# mount -o loop /tmp/image /mnt root@magellan:~# ls -i /mnt 1173 a* 1174 b* 1175 c* 1176 d* 1177 e* 1178 f* 1179 g* 1180 h* root@magellan:~# The inodes were different when mounting the file system the second time. I.e. if one has a windows system with a vfat partition and adds the font directory of the windows system to the search path of fontconfig, the caches in /var/cache/fontconfig will probably appear to be missing after each boot just because the inodes on the windows partition have changed. Maybe using something like realpath() is the best solution after all. Takashi Iwai <tiwai@suse.de> had the idea that realpath can be emulated portably using chdir() and getcwd(). That sounds like a solution which really could work portably. -- Mike FABIAN <mfabian@suse.de> http://www.suse.de/~mfabian ?????????????
Mike FABIAN wrote:> Patrick Lam <plam@MIT.EDU> ????????: > > >>Mike FABIAN wrote: >> >>>Patch by Andreas Schwab <schwab@suse.de>. >>> >>>See also: >>> >>>http://bugzilla.novell.com/show_bug.cgi?id=142215 >> >>Looks good, I''ve committed it. >> >>Are there any of the /var/cache/fontconfig bugs still outstanding? I''ll >>try to take a look at any leftover ones there sooner or later, if folks >>let me know what''s up... > > > I think there are still problems with the normalizing of the font > directory names. > > I posted a patch to this list recently to fix the problem that > > fc-cache /usr/share/fonts > fc-cache /usr/share/fonts/ > cd /usr/share/fonts; fc-cache . > > generate different cache files, but this patch had the problem that > it neede to call FcInitLoadConfigAndFonts () instead of > FcInitLoadConfig () to be able to use FcConfigNormalizeFontDir(). > > But this has the severe disadvantage that FcInitLoadConfigAndFonts () > is very expensive, therefore this is not a good solution.I don''t think that''s true: I think that we *can* call FcConfigNormalizeFontDir if we make sure to call things in the proper order. I haven''t had time recently to investigate this in detail, but hopefully next week. pat
Patrick Lam <plam@MIT.EDU> ????????:> Mike FABIAN wrote: >> >> I think there are still problems with the normalizing of the font >> directory names. >> >> I posted a patch to this list recently to fix the problem that >> >> fc-cache /usr/share/fonts >> fc-cache /usr/share/fonts/ >> cd /usr/share/fonts; fc-cache . >> >> generate different cache files, but this patch had the problem that >> it neede to call FcInitLoadConfigAndFonts () instead of >> FcInitLoadConfig () to be able to use FcConfigNormalizeFontDir(). >> >> But this has the severe disadvantage that FcInitLoadConfigAndFonts () >> is very expensive, therefore this is not a good solution. > > I don''t think that''s true: I think that we *can* call > FcConfigNormalizeFontDir if we make sure to call things in the proper > order. I haven''t had time recently to investigate this in detail, but > hopefully next week.Have you looked into this issue? I still believe that the current approach is flawed because FcInitLoadConfig () doesn''t recurse through the subdirectories and therefore doesn''t initialise the list of font directories used by FcConfigNormalizeFontDir () completely, it fills only the directories directly mentioned in the .conf files into the list. There is another problem: It used to be possible to create a cache file in a directory with fc-cache directory even if this directory was not a subdirectory of a font directory listed in a .conf file. I think this behaviour should not be changed, it should be possible to create a cache in /var/cache/fontconfig for an arbitrary directory by giving it as an argument on the fc-cache commandline. It would be confusing if a cache is only generated if that directory is a subdirectory of a directory listed in a .conf file. But that means that this directory has to be added using FcConfigAddFontDir () in fc-cache, it will never be added automatically no matter which init function is called (neither FcInitLoadConfig () nor FcConfigNormalizeFontDir () will do it). If such a directory is given on the commandline, normalizing it by comparing with the directories in the list is impossible. The only possibility left is to normalize it while adding it. I.e. it would be best if the function FcConfigAddFontDir () would normalize directories while adding them so that they appear in the list always in a standard form. Then we need something like realpath () again. Takashi Iwai <tiwai@suse.de> had the idea that realpath () can be emulated portably using chdir () and getcwd (). I made a patch to use this idea in FcConfigAddFontDir () (patch is attached). Unfortunately we noticed later that this might cause problems because chdir () is process global and therefore not thread-safe. Maybe it is the best to implement portable version of realpath () in fontconfig? libiberty from binutils also implements it''s own version of realpath (), maybe that can be used. -------------- next part -------------- A non-text attachment was scrubbed... Name: improve-normalization-of-font-path.patch Type: text/x-patch Size: 3457 bytes Desc: not available Url : http://lists.freedesktop.org/archives/fontconfig/attachments/20060124/223d6686/improve-normalization-of-font-path.bin -------------- next part -------------- -- Mike FABIAN <mfabian@suse.de> http://www.suse.de/~mfabian ?????????????
Mike FABIAN wrote:> Have you looked into this issue?I was planning to take a look at the open issues today.> It used to be possible to create a cache file in a directory > with > > fc-cache directory > > even if this directory was not a subdirectory of a font directory > listed in a .conf file. I think this behaviour should not be changed, > it should be possible to create a cache in /var/cache/fontconfig for > an arbitrary directory by giving it as an argument on the fc-cache > commandline. It would be confusing if a cache is only generated if > that directory is a subdirectory of a directory listed in a .conf > file.Why would you want to do that? It seems like it would be flawed unless you got a canonical path for the directory anyway. keithp and I talked about implementing our own realpath() a while ago and didn''t especially like the idea. I''ll try to talk to him about it again sometime. I''ll check about FcInitLoadConfig again, but I think that it can be made to add directories. It currently doesn''t in part for globally cached directories, but that''s a separate issue.> Maybe it is the best to implement portable version of realpath () in > fontconfig?I also think it would be really good to get the approach I started working properly, because that really *is* a canonical path to a font. chdir() won''t give you a canonical path, it just gives you a path (e.g. when you have multiple mounts of a directory). pat
Patrick Lam <plam@MIT.EDU> ????????:> Mike FABIAN wrote: >> Have you looked into this issue? > > I was planning to take a look at the open issues today.Thank you!>> It used to be possible to create a cache file in a directory >> with >> >> fc-cache directory >> >> even if this directory was not a subdirectory of a font directory >> listed in a .conf file. I think this behaviour should not be changed, >> it should be possible to create a cache in /var/cache/fontconfig for >> an arbitrary directory by giving it as an argument on the fc-cache >> commandline. It would be confusing if a cache is only generated if >> that directory is a subdirectory of a directory listed in a .conf >> file. > > Why would you want to do that? It seems like it would be flawed unless > you got a canonical path for the directory anyway.Only because it used to work like that and it is a bit confusing if the behaviour changes suddenly. To get information about unknown fonts, I used to put them in some temporary directory and call "fc-cache ." there, then I looked into the cache file. As the caches are now (usually) generated in /var/cache/fontconfig, it has become a bit inconvenient anyway to find out which cache file belongs to which directory, therefore I guess you are right that it doesn''t matter to loose that behaviour. If I want to check what cache files are generated for some new, unknown fonts, I''ll have to add the temporary directory to ~/.fonts.conf now. That''s no big problem, therefore I think you are right that we can forget about directories which are not subdirectories of directories listed in a .conf file. -- Mike FABIAN <mfabian@suse.de> http://www.suse.de/~mfabian ?????????????
Patrick Lam <plam@MIT.EDU> ????????:> I''ll check about FcInitLoadConfig again, but I think that it can be made > to add directories. It currently doesn''t in part for globally cached > directories, but that''s a separate issue.Currently it doesn''t add any subdirectories, only the directories directly listed in .conf files. I''m sure about that. A few days ago, I also had the idea to improve FcInitLoadConfig () to add all subdirectories as well. Maybe that is the way to go because it is in the spirit of the approach you started.>> Maybe it is the best to implement portable version of realpath () in >> fontconfig? > > I also think it would be really good to get the approach I started > working properly, because that really *is* a canonical path to a font. > chdir() won''t give you a canonical path, it just gives you a path (e.g. > when you have multiple mounts of a directory).-- Mike FABIAN <mfabian@suse.de> http://www.suse.de/~mfabian ?????????????
Mike FABIAN wrote:>>Why would you want to do that? It seems like it would be flawed unless >>you got a canonical path for the directory anyway. > > > Only because it used to work like that and it is a bit confusing if > the behaviour changes suddenly. > > To get information about unknown fonts, I used to put them in some > temporary directory and call "fc-cache ." there, then I looked into > the cache file. > > As the caches are now (usually) generated in /var/cache/fontconfig, it > has become a bit inconvenient anyway to find out which cache file > belongs to which directory, therefore I guess you are right that it > doesn''t matter to loose that behaviour. > > If I want to check what cache files are generated for some new, > unknown fonts, I''ll have to add the temporary directory to > ~/.fonts.conf now. > > That''s no big problem, therefore I think you are right that we > can forget about directories which are not subdirectories of > directories listed in a .conf file.What about if fc-cat . did what you wanted? I think that would be helpful, although I''m trying to fix the issues with the cache file directories first; it would be great if someone else made fc-cat work to print out directory font information. pat
Patrick Lam <plam@MIT.EDU> ????????:> Mike FABIAN wrote: >>>Why would you want to do that? It seems like it would be flawed unless >>>you got a canonical path for the directory anyway. >> >> >> Only because it used to work like that and it is a bit confusing if >> the behaviour changes suddenly. >> >> To get information about unknown fonts, I used to put them in some >> temporary directory and call "fc-cache ." there, then I looked into >> the cache file. >> >> As the caches are now (usually) generated in /var/cache/fontconfig, it >> has become a bit inconvenient anyway to find out which cache file >> belongs to which directory, therefore I guess you are right that it >> doesn''t matter to loose that behaviour. >> >> If I want to check what cache files are generated for some new, >> unknown fonts, I''ll have to add the temporary directory to >> ~/.fonts.conf now. >> >> That''s no big problem, therefore I think you are right that we >> can forget about directories which are not subdirectories of >> directories listed in a .conf file. > > What about if fc-cat . did what you wanted?That would be nice!> I think that would be helpful, although I''m trying to fix the issues > with the cache file directories first;Yes, that''s urgent.> it would be great if someone else made fc-cat work to > print out directory font information.-- Mike FABIAN <mfabian@suse.de> http://www.suse.de/~mfabian ?????????????
Patrick Lam <plam@MIT.EDU> ????????: Mike> Maybe it is the best to implement portable version of realpath () in Mike> fontconfig? pat> I also think it would be really good to get the approach I started pat> working properly, because that really *is* a canonical path to a font. pat> chdir() won''t give you a canonical path, it just gives you a path (e.g. pat> when you have multiple mounts of a directory). I ditched the idea of using chdir() and getcwc() and tried to get the approach you started to work properly. Patch attached. I made a new function FcConfigAddFontDirSubdirs (config, dir) which adds all sub directories of "dir" to the list of font directories in "config". Using this function in FcInitLoadConfig () then makes FcInitLoadConfig () initialize the list of font directories correctly without having to resort to the expensive FcInitLoadConfigAndFonts () function. My patch adds some code to check for "." and a final "/" to FcConfigAddFontDir () to make the common cases fc-cache . fc-cache /foo/bar/ work correctly for directories outside of the trees configured in the .conf files as well. Maybe this is not necessary if fc-cat is improved so that it can take a directory as an argument and "fc-cat dir" can give info for any directory. In that case, the checks for "." and "/" can be omitted and the following addition to FcConfigAddFontDir () should be enough: { + if (FcConfigNormalizeFontDir (config, d)) + return FcTrue; /* directory has already been added */ return FcStrSetAddFilename (config->fontDirs, d); } My patch also improves an error message in fcxml.c which just said "out of memory" when a directory could not be added although the reason why the directory could not be added might be something completely different. -------------- next part -------------- A non-text attachment was scrubbed... Name: improve-normalization-of-font-path-add-subdirs.patch Type: text/x-patch Size: 5876 bytes Desc: not available Url : http://lists.freedesktop.org/archives/fontconfig/attachments/20060125/80ef4f73/improve-normalization-of-font-path-add-subdirs.bin -------------- next part -------------- -- Mike FABIAN <mfabian@suse.de> http://www.suse.de/~mfabian ?????????????
Quoting Mike FABIAN <mfabian@suse.de>:> I made a new function FcConfigAddFontDirSubdirs (config, dir) which > adds all sub directories of "dir" to the list of font directories in > "config". > > Using this function in FcInitLoadConfig () then makes FcInitLoadConfig () > initialize the list of font directories correctly without having > to resort to the expensive FcInitLoadConfigAndFonts () function.Hmm, I suspect that this might be too slow on a system with a lot of fonts, because it''s going to have to scan all of the font directories. I''m going to try to patch FcConfigNormalizeFontDir to do a scan if it actually needs to; I think that should limit the hit in performance to the cases where it actually has to do the scan. pat
Patrick Lam <plam@MIT.EDU> ????????:> Hmm, I suspect that this might be too slow on a system with a lot of fonts, > because it''s going to have to scan all of the font directories. I''m going to > try to patch FcConfigNormalizeFontDir to do a scan if it actually > needs to;How can you find out whether it needs too do the scan? I think after FcInitLoadConfig () has been called, the list of directories in "config" should be correct. I.e. the scan is needed in FcInitLoadConfig ().> think that should limit the hit in performance to the cases where it actually > has to do the scan.-- Mike FABIAN <mfabian@suse.de> http://www.suse.de/~mfabian ?????????????
Patrick Lam
2006-Jan-30 08:36 UTC
[Fontconfig] Re: Yet another easy to reproduce crash in fontconfig
Mike FABIAN wrote:> mfabian@magellan:~$ sudo rm /var/cache/fontconfig/*cache* > mfabian@magellan:~$ rm .fonts.cache-2 > mfabian@magellan:~$ fc-match sans > arial.ttf: "Arial" "Regular" > mfabian@magellan:~$ touch .fonts.conf > mfabian@magellan:~$ fc-match sans > ????????????? (core dumped) > mfabian@magellan:~$This bug is also fixed now. pat
Mike FABIAN
2006-Jan-30 12:55 UTC
[Fontconfig] Re: Yet another easy to reproduce crash in fontconfig
Patrick Lam <plam@MIT.EDU> ????????:> Mike FABIAN wrote: >> mfabian@magellan:~$ sudo rm /var/cache/fontconfig/*cache* >> mfabian@magellan:~$ rm .fonts.cache-2 >> mfabian@magellan:~$ fc-match sans >> arial.ttf: "Arial" "Regular" >> mfabian@magellan:~$ touch .fonts.conf >> mfabian@magellan:~$ fc-match sans >> ????????????? (core dumped) >> mfabian@magellan:~$ > > This bug is also fixed now.Yes, thank you! -- Mike FABIAN <mfabian@suse.de> http://www.suse.de/~mfabian ?????????????