[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-2d-dev
Subject:    Re: [OpenJDK 2D-Dev] [9] RFR JDK-7160052: GlyphVector.setGlyphPosition can throw an exception on val
From:       Jayathirth D V <jayathirth.d.v () oracle ! com>
Date:       2015-12-08 7:31:10
Message-ID: d40e5b9d-52d8-4a5c-8b21-23cccac518a6 () default
[Download RAW message or body]

Change looks fine to me.

-----Original Message-----
From: Sergey Bylokhov 
Sent: Monday, December 07, 2015 9:34 PM
To: prasanta sadhukhan; Phil Race; 2d-dev@openjdk.java.net; Rajeev Chamyal
Subject: Re: [OpenJDK 2D-Dev] [9] RFR JDK-7160052: GlyphVector.setGlyphPosition can \
throw an exception on valid input

Thanks for clarification. The fix looks fine then.

On 07.12.15 18:43, prasanta sadhukhan wrote:
> Hi Sergey,
> 
> clearCache(index) is called from 2 places one from setGlyphPositions() 
> which I fixed and other one from setGlyphTransform() and 
> setGlyphTransform is called from public setGlyphTransform() which 
> already have a check for IOB if (ix < 0 || ix >= glyphs.length) {
> throw new IndexOutOfBoundsException("ix = " + ix);
> }
> so this (setGlyphPositions) is the only place where it was called wrongly.
> 
> Regards
> Prasanta
> On 12/7/2015 9:01 PM, Sergey Bylokhov wrote:
> > Hi, Prasanta.
> > Can you please confirm that we properly use clearCaches() in other 
> > places of StandardGlyphVector.java?
> > 
> > On 27.11.15 14:48, prasanta sadhukhan wrote:
> > > Hi All,
> > > 
> > > Please review a fix for jdk9
> > > Bug: https://bugs.openjdk.java.net/browse/JDK-7160052
> > > webrev: http://cr.openjdk.java.net/~psadhukhan/7160052/webrev.00/
> > > 
> > > Issue:
> > > GlyphVector.setGlyphPosition(int glyphIndex, Point2D 
> > > <imap://Sergey%2EBylokhov%40oracle%2Ecom@stbeehive.oracle.com:993/fe
> > > tch%3EUID%3E/mailing%20list/2d%3E5454?header=quotebody&part=1.2.2&fi
> > > lename=Point2D.html>
> > > newPos)
> > > can have glyphIndex "equal" to the number of glyphs in this 
> > > GlyphVector so if user tries to call 
> > > GlyphVector.setGlyphPosition(gv.getNumGlyphs(),
> > > gv.getGlyphPosition(gv.getNumGlyphs()) it throws 
> > > IndexOutOfBoundsException
> > > 
> > > Cause:
> > > StandardGlyphVector maintains a glyph cache for the glyphs stored in 
> > > the GlyphVector via lbcache = new Shape[glyphs.length];
> > > 
> > > When GlyphVector.setGlyphPosition() is called, it positions the 
> > > glyph at the specified glyphIndex|||||||||| and tries to clear the 
> > > mentioned glyphIndex position of cache by calling 
> > > clearCache(glyphIndex) But, if we pass the glyphIndex == number of 
> > > glyphs, then it tries to access beyond the cache array resulting in IOB.
> > > 
> > > Fix:
> > > Checked if glyphIndex passed as argument is equal to number of 
> > > glyphs then do not try to clear the cache as setGlyphPosition() will 
> > > anyway sets the position after the last glyph.
> > > Also, added this check as per spec|IndexOutOfBoundsException 
> > > <imap://Sergey%2EBylokhov%40oracle%2Ecom@stbeehive.oracle.com:993/fe
> > > tch%3EUID%3E/mailing%20list/2d%3E5454?header=quotebody&part=1.2.3&fi
> > > lename=IndexOutOfBoundsException.html>|
> > > 
> > > - if |glyphIndex| is less than 0 or greater than the number of 
> > > glyphs in this |GlyphVector
> > > > 
> > > Regards
> > > Prasanta
> > > 
> > > > > > > 
> > 
> > 
> 


--
Best regards, Sergey.


[prev in list] [next in list] [prev in thread] [next in thread]