Bug 86950

Summary:	Fix or improve docs on FcConfigAppFontAddDir or FcConfigAppFontAddFile return value
Product:	fontconfig	Reporter:	Behdad Esfahbod <freedesktop>
Component:	library	Assignee:	Akira TAGOH <akira>
Status:	RESOLVED FIXED	QA Contact:	Behdad Esfahbod <freedesktop>
Severity:	normal
Priority:	medium	CC:	akira, fontconfig-bugs, freedesktop
Version:	unspecified
Hardware:	Other
OS:	All
Whiteboard:
i915 platform:		i915 features:

Description Behdad Esfahbod 2014-12-03 03:15:06 UTC

These functions return True of memory allocation didn't fail.  This includes the case that the file/directory name passed to them doesn't exist.  This confuses users of the API as they assume that a true return value means that the font(s) were added.

We might be able to fix this to return False if the file/dir doesn't exist.  Might have a few surprises re fc-cache and related API, that might or might not be improvements.

Or at least we should improve docs to note this.

Comment 1 Akira TAGOH 2014-12-09 10:07:33 UTC

Fixed in git.

Comment 2 Behdad Esfahbod 2014-12-09 20:07:17 UTC

Thanks.

Comment 3 Behdad Esfahbod 2014-12-09 20:08:19 UTC

This doesn't make fc-cache /dev/null fail no, does it?  That might actually be a good thing even.  Not sure about directories though.

Comment 4 Akira TAGOH 2014-12-10 02:44:57 UTC

no customers for those APIs in the own tools. it needs to be fixed separately. will work on that too.

Comment 5 Akira TAGOH 2014-12-16 11:45:21 UTC

fixed in git for fc-cache too

Comment 6 Behdad Esfahbod 2014-12-16 21:07:44 UTC

Well, I guess we'll wait until distro people nag about fc-cache behavior having been changed :).  Thanks.

Comment 7 Akira TAGOH 2014-12-17 10:28:15 UTC

I got a complaint about it already. was about to add --ignore-fail-on-no-fonts-dir option to get back the old behavior.

Comment 8 Behdad Esfahbod 2014-12-17 17:43:08 UTC

I think we might to keep old behavior by default.  What do you think?

Comment 9 Akira TAGOH 2014-12-19 07:46:49 UTC

No strong opinion for that decision though, we may try to raise an error if the given directory can't be opened perhaps. i.e. handle an error differently for no-fonts-in-directory and no-such-directories or unable-to-open-a-directory. so we can inform someone there are a sort of typos there.

Comment 10 Behdad Esfahbod 2015-06-27 00:06:05 UTC

After the changes broke many clients, I reverted the AddDir() changes and left AddFont() in.

commit 46ec6a52d4cc447cc3ff4a13b2067ecb76c9db2e
Author: Behdad Esfahbod <behdad@behdad.org>
Date:   Fri Jun 26 17:02:13 2015 -0700

    Revert changes made to FcConfigAppFontAddDir() recently
    
    In 32ac7c75e8db0135ef37cf86f92d8b9be000c8bb the behavior of
    FcConfigAppFontAddFile/Dir() were changed to return false
    if not fonts were found.  While this is welldefined and useful
    for AddFile(), it's quite problematic for AddDir().  For example,
    if the directory is empty, is that a failure or success?  Worse,
    the false value from AddDir() was being propagated all the way
    to FcInit() returning false now.  This only happened upon memory
    allocation failure before, and some clients assert that FcInit()
    is successful.
    
    With this change, AddDir() is reverted back to what it was.
    AddFont() change (which was actually in fcdir.c) from the original
    commit is left in.

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.