If that's the case, then this is a terrible API.  If the function returns NULL, 
the caller has no information about the state of its passed pattern.  It may 
have been deallocated and NULL returned during 'goto bail' or it may not have 
been deallocated and NULL returned because of invalid paramaters (currently 
just info == NULL).

Since the function isn't documented, I wonder if the "fix" would be to copy 
pattern and write documentation to this effect.  The problem would then be a 
leak for those expecting it to be freed, but that is certainly preferred, IMO.  
Also, the leak is easily fixed by requiring the new libXft and doing their own 
FcPatternDestroy.

This confusion has caused problems in external projects who expect it to not be 
freed.  The only non-xorg hit for XftFontOpenInfo in codesearch is actually a 
long comment debugging this issue and removing their FcPatternDestroy:

http://google.com/codesearch#Lbvu5g5_Qs8/mrxvt-0.5.2/src/main.c&q=XftFontOpenInfo%20lang:%5Ec$&type=cs


On Oct 3, 2011, at 09:53, Alan Coopersmith wrote:

> On 10/ 2/11 02:55 PM, Jeremy Huddleston wrote:
>> 
>> https://bugs.freedesktop.org/show_bug.cgi?id=5745
>> 
>> Signed-off-by: Jeremy Huddleston<[email protected]>
>> ---
>>  src/xftfreetype.c |    1 -
>>  1 files changed, 0 insertions(+), 1 deletions(-)
>> 
>> diff --git a/src/xftfreetype.c b/src/xftfreetype.c
>> index 3f8dfef..1c5967a 100644
>> --- a/src/xftfreetype.c
>> +++ b/src/xftfreetype.c
>> @@ -798,7 +798,6 @@ XftFontOpenInfo (Display *dpy,
>>      {
>>          if (!font->ref++)
>>              --info->num_unref_fonts;
>> -        FcPatternDestroy (pattern);
>>          return&font->public;
>>      }
>> 
> 
> I'm not sure, since if it passes that point, it saves the pattern in
> the newly created font's font->public.pattern and then does the
> FcPatternDestroy of it in XftFontDestroy.
> 
> Perhaps we just need to document that the pattern passed to XftFontOpenInfo
> becomes the exclusive property of Xft and the caller is not allowed to use
> it any more?
> 
> (Though it seems XftFontOpenInfo is missing from the function list in the
> Xft man page, so needs more love than that.)
> 
> -- 
>       -Alan Coopersmith-        [email protected]
>        Oracle Solaris Platform Engineering: X Window System
> 
> _______________________________________________
> [email protected]: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
> 

_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Reply via email to