[Fontconfig] [PATCH] fc-list: Exit with an error for invalid patterns

[Behdad Esfahbod]

On 13-09-24 10:09 PM, Akira TAGOH wrote:
> fc-list is designed to return the errror code only when --quiet option
> is set and no fonts matched. otherwise always 0. well, at least at
> this moment. so that is the expected behavior and you should see same
> result on box where no fonts installed.

But if the input pattern is invalid, that's good reason to return an error
code even without --quiet.  It's a usage error.  Plus, returning 0 and no
fonts means "no fonts matched", where in fact the pattern was invalid.


> On Wed, Sep 25, 2013 at 1:12 AM, W. Trevor King <wking at tremily.us> wrote:
>> On Tue, Sep 24, 2013 at 06:01:58PM +0900, Akira TAGOH wrote:
>>> On Tue, Sep 24, 2013 at 1:06 PM, W. Trevor King <wking at tremily.us> wrote:
>>>> That would be true if FcListPatternMatchAny wasn't an internal
>>>> function:
>>>>
>>>>   $ git --no-pager grep FcListPatternMatchAny
>>>>   src/fccfg.c:    if (FcListPatternMatchAny (patterns->fonts[i], font))
>>>>   src/fcint.h:FcListPatternMatchAny (const FcPattern *p,
>>>>   src/fclist.c:FcListPatternMatchAny (const FcPattern *p,
>>>>   src/fclist.c:       if (FcListPatternMatchAny (p,               /* pattern */
>>>>
>>>> I'm also not sure it makes sense to have different handling for NULL
>>>> FcPatterns and non-NULL FcPatterns with no elements, since the comment
>>>> for FcListPatternMatchAny says:
>>>
>>> Talking about NULL FcPattern being introduced by FcNameParse() would
>>> means no valid patterns. given that holding that state and calling
>>> FcFontList() would ends up to the same result because of no such fonts
>>> matched to that pattern. and usually not expecting to see NULL pattern
>>> to any APIs. they should use the empty FcPattern instead.
>>
>> Ok, it looks like you want different handling for NULL and empty
>> patterns.  I was just making sure it was a conscious decision ;).
>>
>>> Or applications shouldn't simply continue to work if FcNameParse()
>>> failed in this case.
>>
>> And that's what the !pat check in fc-match.c, fc-pattern.c, and my
>> patch for fc-list.c does ;).  Maybe we want both my patch (actively
>> die in fc-list on bad patterns and out-of-memory errors) and yours
>> (don't crash if a programmer forgets to check for bad patterns or
>> out-of-memory errors)?  One benefit would be that invalid patterns
>> return an error code, while the new master does not:
>>
>>   $ LD_LIBRARY_PATH=src/.libs/ ./fc-list/.libs/fc-list :charset=xyz
>>   $ echo $?
>>   0
>>
>> since it thinks it successfully searched for the unmatchable NULL
>> pattern.
>>
>> Cheers,
>> Trevor
>>
>> --
>> This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
>> For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
> 
> 
> 

-- 
behdad
http://behdad.org/