bugThe FreeType Project - Bugs: bug #27921, CFF boundingbox is wrong when max...

 
 

bug #27921: CFF boundingbox is wrong when max value is negative.

Submitted by:  bram tassyns <bramt>
Submitted on:  Tue 03 Nov 2009 08:06:16 AM UTC  
 
Severity: 3 - NormalItem Group: Incorrect behaviour
Status: FixedPrivacy: Public
Assigned to: Werner LEMBERG <wl>Open/Closed: Closed
Planned Release: 2.3.12

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

Wed 04 Nov 2009 09:07:09 AM UTC, comment #3:

Fixed in git. Please test.

Werner LEMBERG <wl>
Project AdministratorIn charge of this item.
Wed 04 Nov 2009 08:32:19 AM UTC, comment #2:

Yes that fixes the issue as well.

bram tassyns <bramt>
Wed 04 Nov 2009 07:14:31 AM UTC, comment #1:

What about using

cffface->bbox.xMax = ( dict->font_bbox.xMax + 0xFFFF ) >> 16;

instead, together with a remark that no `U' suffix must be appended?

Regarding the `>> 16' I fully agree that it isn't portable officially, but noone has ever complained until now...

This page:

https://www.securecoding.cert.org/confluence/display/seccode/INT34-C.+Do+not+shift+a+negative+number+of+bits+or+more+bits+than+exist+in+the+operand

gives a nice overview, BTW.

I suggest not to take any action right now.

Werner LEMBERG <wl>
Project AdministratorIn charge of this item.
Tue 03 Nov 2009 08:06:16 AM UTC, original submission:

cffobjs.c, line 662:

cffface->bbox.xMax = ( dict->font_bbox.xMax + 0xFFFFU ) >> 16;
cffface->bbox.yMax = ( dict->font_bbox.yMax + 0xFFFFU ) >> 16;

This goes wrong when xMax or yMax contains a negative value (note that this case isn't very likely to happen).

Fix:

cffface->bbox.xMax = ( dict->font_bbox.xMax + (FT_Int32) 0xFFFFU ) >> 16;
cffface->bbox.yMax = ( dict->font_bbox.yMax + (FT_Int32) 0xFFFFU ) >> 16;

As a side question, using the >> 16 operator on a negative value is (as far as i could find) undefined behaviour (the c++ spec actually defines it as being undefined). However this happens alot throughout the code. Is this really save ? (or should it be replaced by something like / 65536) ? I should note that i haven't found any compiler yet that doesn't implement it in the way FT uses it.

bram tassyns <bramt>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by wl (Posted a comment)
  • -unavailable- added by bramt (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 5 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 04 Nov 2009 09:07:09 AM UTCwlStatusIn Progress=>Fixed
      Open/ClosedOpen=>Closed
      Planned ReleaseNone=>2.3.12
    Wed 04 Nov 2009 07:14:31 AM UTCwlAssigned toNone=>wl
    Wed 04 Nov 2009 07:14:30 AM UTCwlStatusNone=>In Progress

    Back to the top


    Powered by Savane 3.1-cleanup1