Closed Bug 368668 Opened 13 years ago Closed 13 years ago

coverity CID: 1106 deadcode at cairo-cff-subset.c:encode_integer

Categories

(Core :: Graphics, defect)

x86
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla1.9alpha8

People

(Reporter: guninski, Assigned: ehsan)

References

(Blocks 1 open bug, )

Details

(Keywords: coverity)

Attachments

(1 file)

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/gfx/cairo/cairo/src/cairo-cff-subset.c&rev=1.1&mark=167,171#168

 [167]                  } else if (i >= -1131 && i <= -108) {

 168                      i = -i - 108;
 169 vladimir 1.1         *p++ = (i >> 8)+ 251;
 170                      *p++ = i & 0xff;

 [171]                  } else if (i >= -1131 && i <= -108) {

if the condition at [167] is false the condition at [171] is also false.
if the condition at [167] is true the [171] is not reached because of else.

in addition this function seems to do uselesss operation on |i|.



 172                      *p++ = 28;
Assignee: general → general
Component: General → GFX
Keywords: coverity
Product: Mozilla Application Suite → Core
QA Contact: general → ian
Assignee: general → nobody
Component: GFX → GFX: Thebes
QA Contact: ian → thebes
What should a patch do here?  Just remove the "else if" at line 171?
(In reply to comment #1)
> What should a patch do here?  Just remove the "else if" at line 171?
> 

not quite sure, can't get the context. it is possible that the condition must be different and the block present.
I have fixed this bug in cairo 1.5.1
http://gitweb.freedesktop.org/?p=cairo;a=commit;h=f52aa4c13e91339e575ca2c52c9e3a1f4d95b106

Line 171 should be 
    } else if (i >= -32768 && i <= 32767) {

The effect of this bug is that some integers are encoded with five bytes instead of three resulting in a slightly larger subsetted font.
Target Milestone: --- → mozilla1.9 M8
Keywords: checkin-needed
Whiteboard: [checkin needed]
Whiteboard: [checkin needed]
Near as I can tell from the clear-as-mud approval rules, you need approval1.9+ before you're checkin-needed.
Keywords: checkin-needed
Attachment #275092 - Flags: approval1.9?
vlad: can you approve this/push it upstream?
(In reply to comment #6)
> vlad: can you approve this/push it upstream?

The patch is my patch from upstream.

As the bug doesn't actually break anything (the effect is about a 1% increase in embedded font size in PDFs) I would suggest waiting for the next cairo upgrade to pull in the fix.
(In reply to comment #7)
> As the bug doesn't actually break anything (the effect is about a 1% increase
> in embedded font size in PDFs) I would suggest waiting for the next cairo
> upgrade to pull in the fix.

So, should we close this bug?
Attachment #275092 - Flags: approval1.9?
Yes, fixed by bug 395449.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.