GNOME Bugzilla – Bug 565379
pango_font_description_better_match some style attributes of old_match are not checked
Last modified: 2008-12-25 18:38:43 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:
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.
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.
Created attachment 125187 [details] [review] a few changes in function implementation
(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.
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?
(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.
OK. Thanks for information.