Summary: | Running 'fc-match --all' core dumps when no fonts are installed | ||
---|---|---|---|
Product: | fontconfig | Reporter: | Arvind Umrao <arvind.umrao> |
Component: | fc-match | Assignee: | Arvind Umrao <arvind.umrao> |
Status: | RESOLVED FIXED | QA Contact: | Behdad Esfahbod <freedesktop> |
Severity: | normal | ||
Priority: | medium | CC: | akira, freedesktop |
Version: | 2.8 | ||
Hardware: | All | ||
OS: | All | ||
Whiteboard: | |||
i915 platform: | i915 features: |
Description
Arvind Umrao
2011-08-29 02:43:43 UTC
*** Bug 40453 has been marked as a duplicate of this bug. *** Proposed fix for this: http://cgit.freedesktop.org/~tagoh/fontconfig/commit/?h=bz40452&id=08bfad59a670ec817910d17baab09ff5c69c66b2 fc-match -s and fc-match -a works without the segfault then. It is better to give some sort of error messages. proposed fixes diff --git a/fc-match/fc-match.c b/fc-match/fc-match.c index e64b4bc..8b59896 100644 --- a/fc-match/fc-match.c +++ b/fc-match/fc-match.c @@ -175,6 +175,12 @@ main (int argc, char **argv) int j; font_patterns = FcFontSort (0, pat, all ? FcFalse : FcTrue, 0, &result); + if (!font_patterns) + { + fprintf (stderr, "Can't find any matching font\n"); + return 1; + } + for (j = 0; j < font_patterns->nfont; j++) { FcPattern *font_pattern; -- 1.7.6 How about this: diff --git a/src/fcmatch.c b/src/fcmatch.c index 1b9162b..95ed898 100644 --- a/src/fcmatch.c +++ b/src/fcmatch.c @@ -686,7 +686,7 @@ FcFontSetSort (FcConfig *config, nnodes += s->nfont; } if (!nnodes) - goto bail0; + return FcFontSetCreate (); for (nPatternLang = 0; FcPatternGet (p, FC_LANG, nPatternLang, &patternLang) == FcResultMatch; (In reply to comment #4) > How about this: > > diff --git a/src/fcmatch.c b/src/fcmatch.c > index 1b9162b..95ed898 100644 > --- a/src/fcmatch.c > +++ b/src/fcmatch.c > @@ -686,7 +686,7 @@ FcFontSetSort (FcConfig *config, > nnodes += s->nfont; > } > if (!nnodes) > - goto bail0; > + return FcFontSetCreate (); > > for (nPatternLang = 0; > FcPatternGet (p, FC_LANG, nPatternLang, &patternLang) == > FcResultMatch; If we handle null condition at fc-match/fc-match.c, we can give error message to user. At src/fcmatch.c, it would be difficult to give error message. I can see at many other place in fc-match.c file null condition has been checked. We sure should check for NULLs there, but FcFontSort() should not return NULL if no fonts are found. FcFontMatch should I guess. (In reply to comment #6) > We sure should check for NULLs there, but FcFontSort() should not return NULL > if no fonts are found. FcFontMatch should I guess. I think bail out was right, because no font installed in system. (In reply to comment #6) > We sure should check for NULLs there, but FcFontSort() should not return NULL > if no fonts are found. FcFontMatch should I guess. Well, IMHO if FcFontSet *sets is NULL, FcFontSetSort should returns NULL as well. thus that makes sense (in this case at least) that FcFontSort() gives a pointer of FcFontSet that contains a reference of FcFontSet in FcConfig. We need fixes in multiple places. trivial updates after looking at other source code: http://cgit.freedesktop.org/~tagoh/fontconfig/commit/?h=bz40452&id=4c4733f292433affb02645cafeff1ef785ab7671 FWIW this change may breaks firefox perhaps. they are only relying on the null result for validation. so this may causes the unexpected result. Though there seems no problem on Pango and Qt unless I'm missing anything else. (In reply to comment #10) > trivial updates after looking at other source code: > http://cgit.freedesktop.org/~tagoh/fontconfig/commit/?h=bz40452&id=4c4733f292433affb02645cafeff1ef785ab7671 > > FWIW this change may breaks firefox perhaps. they are only relying on the null > result for validation. so this may causes the unexpected result. > > Though there seems no problem on Pango and Qt unless I'm missing anything else. You might want to assert result before if(result) condition check. like this way @@ -672,6 +683,19 @@ FcFontSetSort (FcConfig *config, FcBool *patternLangSat; FcValue patternLang; + assert (sets != NULL); + assert (p != NULL); + assert (result != NULL); + /* There are some implementation that relying on the result of + * "result" to check if the return value of FcFontSetSort + * is valid or not. + * So we should initialize it to the conservative way since + * this function doesn't return NULL anymore. + */ + if (result) + *result = FcResultNoMatch; + not a big deal. either way should works. Fixed in a18ca17b. |
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.