After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 565379 - pango_font_description_better_match some style attributes of old_match are not checked
pango_font_description_better_match some style attributes of old_match are no...
Status: RESOLVED FIXED
Product: pango
Classification: Platform
Component: general
1.22.x
Other All
: Normal normal
: ---
Assigned To: pango-maint
pango-maint
Depends on:
Blocks:
 
 
Reported: 2008-12-22 20:08 UTC by Michael Smirnov
Modified: 2008-12-25 18:38 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
a few changes in function implementation (759 bytes, patch)
2008-12-23 10:07 UTC, Michael Smirnov
rejected Details | Review

Description Michael Smirnov 2008-12-22 20:08:53 UTC
Please describe the problem:
If new_match is a match for desc and old_match is not then the function should return TRUE for new_match is better match for desc than old_match. But there is a way for function to return FALSE in this situation, because there is no check for variant, stretch and gravity of old_match to be equal to corresponding fields of desc.

Steps to reproduce:
PangoFontDescription *desc      = NULL;
PangoFontDescription *old_match = NULL;
PangoFontDescription *new_match = NULL;
    
desc      = pango_font_description_new();
old_match = pango_font_description_new();
new_match = pango_font_description_new();
    
pango_font_description_set_stretch(old_match, PANGO_STRETCH_EXPANDED);

pango_font_description_better_match(desc, old_match, new_match);

Actual results:
pango_font_description_better_match returns FALSE instead of TRUE

Expected results:
pango_font_description_better_match should return TRUE

Does this happen every time?
Sure

Other information:
Comment 1 Behdad Esfahbod 2008-12-22 21:15:37 UTC
I added a note that @old_match must be a match for @desc.

2008-12-22  Behdad Esfahbod  <behdad@gnome.org>

        Bug 565379 – pango_font_description_better_match some style attributes
        of old_match are not checked

        * pango/fonts.c: Improve docs.

Comment 2 Michael Smirnov 2008-12-23 10:07:16 UTC
Hi Behdad,

I'm just a little curious why not to solve this by adding checks for style attributes that must match?

IMHO this would be an elegant way of solving this isuue. By this checks all cases will be covered and there will be no undefined behavior.

To be more specific sending you the patch.
Comment 3 Michael Smirnov 2008-12-23 10:07:53 UTC
Created attachment 125187 [details] [review]
a few changes in function implementation
Comment 4 Behdad Esfahbod 2008-12-23 18:31:36 UTC
(In reply to comment #2)
> Hi Behdad,
> 
> I'm just a little curious why not to solve this by adding checks for style
> attributes that must match?

Because they are not needed.  You are not supposed to pass random stuff into this function.  old_match is your previous best-match.  So, it's a match.


> IMHO this would be an elegant way of solving this isuue. By this checks all
> cases will be covered and there will be no undefined behavior.

My opinion of what is elegant is different from yours. :)  Adding code for the sake of it is not elegant.

> To be more specific sending you the patch.
Comment 5 Michael Smirnov 2008-12-25 17:34:30 UTC
Hello Behdad

(In reply to comment #4)
> You are not supposed to pass random stuff into
> this function.  old_match is your previous best-match.  So, it's a match.

Passing PangoFontDescription structure to this function is not passing random stuff. What if I do not have a best match, but have 3 descriptions and want to know which of last two is the better match for the first one? Description states "Determines if the style attributes of new_match are a closer match for desc than those of old_match are." With this description it is natural to implement what I said before.

But hey, it's your call.

By the way, do I need to reopen a bug for you to get new comments?
Comment 6 Behdad Esfahbod 2008-12-25 18:34:00 UTC
(In reply to comment #5)
> Hello Behdad
> 
> (In reply to comment #4)
> > You are not supposed to pass random stuff into
> > this function.  old_match is your previous best-match.  So, it's a match.
> 
> Passing PangoFontDescription structure to this function is not passing random
> stuff.

It is according to the docs now :).

> What if I do not have a best match, but have 3 descriptions and want to
> know which of last two is the better match for the first one? Description
> states "Determines if the style attributes of new_match are a closer match for
> desc than those of old_match are." With this description it is natural to
> implement what I said before.

The use case for this function is very limited (to choose the best match among a list) and I don't see any reason to change that.

> But hey, it's your call.
> 
> By the way, do I need to reopen a bug for you to get new comments?

No.
Comment 7 Michael Smirnov 2008-12-25 18:38:43 UTC
OK. Thanks for information.