[Fontconfig] Re: Bug in fc-cat with user cache

[Mike FABIAN]

Mike FABIAN <mfabian at suse.de> さんは書きました:

> Patrick Lam <plam at MIT.EDU> さんは書きました:
>
>> Mike FABIAN wrote:
>>> Yes, I can confirm that it is still incorrect like Frederic writes.
>>> What was wrong with my attached patch? I'm not sure
>>> but I think something similar is necessary. 	    if
>>> (!FcDirCacheConsume (cache->fd, dir, set, config))
>>> always adds the top-level directory to the list of directories
>>> which is then used in FcConfigNormalizeFontDir(). For example it adds
>>> the same directory /usr/X11R6/lib/X11/fonts/ many times but never
>>> the subdirectories like /usr/X11R6/lib/X11/fonts/Type1/ ...
>>> Replacing "dir" by "d->name" like
>>> 	    if (!FcDirCacheConsume (cache->fd, d->name, set, config))
>>> seems to fix this because now the correct directory names area added
>>> to the list.
>>
>> I don't think it should be necessary if you don't normalize the path
>> in the outer fc-cat loop, because you have to treat the directory
>> anyway before you can emit the cache, which should cause the proper
>> directory to be added to fontDirs.  Clearly the present code is
>> incorrect, and I'll look at it (since I'm now back in civilization),
>> but I think that we can avoid the issues involved with your patch.
>>
>> Also, if d->name and dir don't match, there's something wrong...
>
> Then why is the comparison done like this in FcGlobalCacheReadDir():
>
>     if (strncmp (d->name, dir, strlen(dir)) == 0)
>
> ?
>
> That checks whether "d->name" is a subdirectory of "dir".
>
> If "d->name" and "dir" have to match here, the comparison
> should be
>
>     if (strcmp (d->name, dir) == 0)
>
> But then it would almost always fail, except for the top-level
> directory.
>
> Can't you reproduce the following problem?:

[... procedure how to reproduce ...]

When reading a user specific cache like $HOME/.fonts.cache-2, the
cache may contain information for many font directories.

For that reason the code in FcGlobalCacheReadDir() loops over
all dirs found in "cache".

    for (d = cache->dirs; d; d = d->next)

Therefore, the directory given within that loop as a parameter to the
FcDirCacheConsume() call must be the directory from the current value
of "d" in that loop, i.e. "d->name".

Therefore I believe my patch is correct and I cannot find any issues
with that patch. I just fixes the problem described in my last mail.

-- 
Mike FABIAN   <mfabian at suse.de>   http://www.suse.de/~mfabian
睡眠不足はいい仕事の敵だ。