[PATCH] Fix Use ArrayList.add() in StyleSheet.resolveStyle

View: New views
6 Messages — Rating Filter:   Alert me  

[PATCH] Fix Use ArrayList.add() in StyleSheet.resolveStyle

by Pekka Enberg-4 :: Rate this Message:

| View Threaded | Show Only this Message

The following cleanup patch:

    More warning fixes.

    2009-03-09  Andrew John Hughes  <ahughes@...>

[snip]

        * javax/swing/text/html/StyleSheet.java:
        Add generic typing.

changed the code to do ArrayList.set() on an instance thats allocated like this:

    List<Map<String,String>> attributes =
      new ArrayList<Map<String,String>>(count);

This is, however, broken as ArrayList constructor only ensures capacity but
doesn't allow you to set() elements outside of ArrayList.size(). This causes
the following exception to happen upon JPC start-up:

penberg@jaguar:~/testing/jato$ /usr/local/jamvm/bin/jamvm -jar JPCApplication.jar
Exception in thread "main" java.lang.ExceptionInInitializerError
   at java.lang.VMClass.forName(Native Method)
   at java.lang.Class.forName(Class.java:233)
   at jamvm.java.lang.JarLauncher.main(JarLauncher.java:46)
Caused by: java.lang.IndexOutOfBoundsException: Index: 0, Size: 0
   at java.util.ArrayList.raiseBoundsError(ArrayList.java:504)
   at java.util.ArrayList.checkBoundExclusive(ArrayList.java:490)
   at java.util.ArrayList.set(ArrayList.java:323)
   at javax.swing.text.html.StyleSheet.resolveStyle(StyleSheet.java:417)
   at javax.swing.text.html.StyleSheet.getResolvedStyle(StyleSheet.java:376)
   at javax.swing.text.html.StyleSheet.getRule(StyleSheet.java:358)
   at javax.swing.text.html.ViewAttributeSet.<init>(ViewAttributeSet.java:112)
   at javax.swing.text.html.StyleSheet.getViewAttributes(StyleSheet.java:562)
   at javax.swing.text.html.ParagraphView.getAttributes(ParagraphView.java:117)
   at javax.swing.text.html.ParagraphView.setPropertiesFromAttributes(ParagraphView.java:131)
   at javax.swing.text.html.ParagraphView.setParent(ParagraphView.java:106)
   at javax.swing.text.CompositeView.replace(CompositeView.java:214)
   at javax.swing.text.BoxView.replace(BoxView.java:232)
   at javax.swing.text.html.BlockView.replace(BlockView.java:665)
   at javax.swing.text.CompositeView.loadChildren(CompositeView.java:123)
   at javax.swing.text.CompositeView.setParent(CompositeView.java:138)
   at javax.swing.text.html.BlockView.setParent(BlockView.java:187)
   at javax.swing.text.CompositeView.replace(CompositeView.java:214)
   at javax.swing.text.BoxView.replace(BoxView.java:232)
   at javax.swing.text.html.BlockView.replace(BlockView.java:665)
   at javax.swing.text.CompositeView.loadChildren(CompositeView.java:123)
   at javax.swing.text.CompositeView.setParent(CompositeView.java:138)
   at javax.swing.text.html.BlockView.setParent(BlockView.java:187)
   at javax.swing.plaf.basic.BasicTextUI$RootView.setView(BasicTextUI.java:322)
   at javax.swing.plaf.basic.BasicTextUI.setView(BasicTextUI.java:1499)
   at javax.swing.plaf.basic.BasicTextUI.modelChanged(BasicTextUI.java:1522)
   at javax.swing.plaf.basic.BasicTextUI$Handler.propertyChange(BasicTextUI.java:210)
   at java.beans.PropertyChangeSupport.firePropertyChange(PropertyChangeSupport.java:388)
   at java.beans.PropertyChangeSupport.firePropertyChange(PropertyChangeSupport.java:332)
   at java.awt.Component.firePropertyChange(Component.java:5236)
   at javax.swing.text.JTextComponent.setDocument(JTextComponent.java:1343)
   at javax.swing.JEditorPane.setEditorKit(JEditorPane.java:1081)
   at javax.swing.JEditorPane.<init>(JEditorPane.java:715)
   at org.jpc.j2se.JPCApplication.<clinit>(JPCApplication.java:55)
   at java.lang.VMClass.forName(Native Method)
   ...2 more

Signed-off-by: Pekka Enberg <penberg@...>
---
 javax/swing/text/html/StyleSheet.java |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/javax/swing/text/html/StyleSheet.java b/javax/swing/text/html/StyleSheet.java
index 5cf015b..31879b2 100644
--- a/javax/swing/text/html/StyleSheet.java
+++ b/javax/swing/text/html/StyleSheet.java
@@ -414,11 +414,12 @@ public class StyleSheet extends StyleContext
               tags[i] = t.toString();
             else
               tags[i] = null;
-            attributes.set(i, attributeSetToMap(atts));
+            attributes.add(attributeSetToMap(atts));
           }
         else
           {
             tags[i] = null;
+            attributes.add(null);
           }
       }
     tags[0] = tag.toString();
--
1.7.1



Re: [PATCH] Fix Use ArrayList.add() in StyleSheet.resolveStyle

by Andrew Hughes-7 :: Rate this Message:

| View Threaded | Show Only this Message

On 22:07 Thu 10 Mar     , Pekka Enberg wrote:

> The following cleanup patch:
>
>     More warning fixes.
>
>     2009-03-09  Andrew John Hughes  <ahughes@...>
>
> [snip]
>
>         * javax/swing/text/html/StyleSheet.java:
>         Add generic typing.
>
> changed the code to do ArrayList.set() on an instance thats allocated like this:
>
>     List<Map<String,String>> attributes =
>       new ArrayList<Map<String,String>>(count);
>
> This is, however, broken as ArrayList constructor only ensures capacity but
> doesn't allow you to set() elements outside of ArrayList.size(). This causes
> the following exception to happen upon JPC start-up:
>
> penberg@jaguar:~/testing/jato$ /usr/local/jamvm/bin/jamvm -jar JPCApplication.jar
> Exception in thread "main" java.lang.ExceptionInInitializerError
>    at java.lang.VMClass.forName(Native Method)
>    at java.lang.Class.forName(Class.java:233)
>    at jamvm.java.lang.JarLauncher.main(JarLauncher.java:46)
> Caused by: java.lang.IndexOutOfBoundsException: Index: 0, Size: 0
>    at java.util.ArrayList.raiseBoundsError(ArrayList.java:504)
>    at java.util.ArrayList.checkBoundExclusive(ArrayList.java:490)
>    at java.util.ArrayList.set(ArrayList.java:323)
>    at javax.swing.text.html.StyleSheet.resolveStyle(StyleSheet.java:417)
>    at javax.swing.text.html.StyleSheet.getResolvedStyle(StyleSheet.java:376)
>    at javax.swing.text.html.StyleSheet.getRule(StyleSheet.java:358)
>    at javax.swing.text.html.ViewAttributeSet.<init>(ViewAttributeSet.java:112)
>    at javax.swing.text.html.StyleSheet.getViewAttributes(StyleSheet.java:562)
>    at javax.swing.text.html.ParagraphView.getAttributes(ParagraphView.java:117)
>    at javax.swing.text.html.ParagraphView.setPropertiesFromAttributes(ParagraphView.java:131)
>    at javax.swing.text.html.ParagraphView.setParent(ParagraphView.java:106)
>    at javax.swing.text.CompositeView.replace(CompositeView.java:214)
>    at javax.swing.text.BoxView.replace(BoxView.java:232)
>    at javax.swing.text.html.BlockView.replace(BlockView.java:665)
>    at javax.swing.text.CompositeView.loadChildren(CompositeView.java:123)
>    at javax.swing.text.CompositeView.setParent(CompositeView.java:138)
>    at javax.swing.text.html.BlockView.setParent(BlockView.java:187)
>    at javax.swing.text.CompositeView.replace(CompositeView.java:214)
>    at javax.swing.text.BoxView.replace(BoxView.java:232)
>    at javax.swing.text.html.BlockView.replace(BlockView.java:665)
>    at javax.swing.text.CompositeView.loadChildren(CompositeView.java:123)
>    at javax.swing.text.CompositeView.setParent(CompositeView.java:138)
>    at javax.swing.text.html.BlockView.setParent(BlockView.java:187)
>    at javax.swing.plaf.basic.BasicTextUI$RootView.setView(BasicTextUI.java:322)
>    at javax.swing.plaf.basic.BasicTextUI.setView(BasicTextUI.java:1499)
>    at javax.swing.plaf.basic.BasicTextUI.modelChanged(BasicTextUI.java:1522)
>    at javax.swing.plaf.basic.BasicTextUI$Handler.propertyChange(BasicTextUI.java:210)
>    at java.beans.PropertyChangeSupport.firePropertyChange(PropertyChangeSupport.java:388)
>    at java.beans.PropertyChangeSupport.firePropertyChange(PropertyChangeSupport.java:332)
>    at java.awt.Component.firePropertyChange(Component.java:5236)
>    at javax.swing.text.JTextComponent.setDocument(JTextComponent.java:1343)
>    at javax.swing.JEditorPane.setEditorKit(JEditorPane.java:1081)
>    at javax.swing.JEditorPane.<init>(JEditorPane.java:715)
>    at org.jpc.j2se.JPCApplication.<clinit>(JPCApplication.java:55)
>    at java.lang.VMClass.forName(Native Method)
>    ...2 more
>
> Signed-off-by: Pekka Enberg <penberg@...>
> ---
>  javax/swing/text/html/StyleSheet.java |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/javax/swing/text/html/StyleSheet.java b/javax/swing/text/html/StyleSheet.java
> index 5cf015b..31879b2 100644
> --- a/javax/swing/text/html/StyleSheet.java
> +++ b/javax/swing/text/html/StyleSheet.java
> @@ -414,11 +414,12 @@ public class StyleSheet extends StyleContext
>                tags[i] = t.toString();
>              else
>                tags[i] = null;
> -            attributes.set(i, attributeSetToMap(atts));
> +            attributes.add(attributeSetToMap(atts));
>            }
>          else
>            {
>              tags[i] = null;
> +            attributes.add(null);
>            }
>        }
>      tags[0] = tag.toString();
> --
> 1.7.1
>
>

Thanks for catching this bug.  I was going to approve it but you seem to have already committed
it -- why???
--
Andrew :)

Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

Support Free Java!
Contribute to GNU Classpath and IcedTea
http://www.gnu.org/software/classpath
http://icedtea.classpath.org
PGP Key: F5862A37 (https://keys.indymedia.org/)
Fingerprint = EA30 D855 D50F 90CD F54D  0698 0713 C3ED F586 2A37


Re: [PATCH] Fix Use ArrayList.add() in StyleSheet.resolveStyle

by Pekka Enberg-4 :: Rate this Message:

| View Threaded | Show Only this Message

Hi Andrew!

On Mon, Mar 14, 2011 at 11:46 PM, Dr Andrew John Hughes
<ahughes@...> wrote:
> Thanks for catching this bug.  I was going to approve it but you seem to have already committed
> it -- why???

I thought that'd be OK since the fix was pretty simple and I was able
to work out why the regression was there. Would you prefer that I wait
for an explicit ACKs in the future?

                        Pekka


Re: [PATCH] Fix Use ArrayList.add() in StyleSheet.resolveStyle

by Andrew Hughes-7 :: Rate this Message:

| View Threaded | Show Only this Message

On 14 March 2011 21:56, Pekka Enberg <penberg@...> wrote:

> Hi Andrew!
>
> On Mon, Mar 14, 2011 at 11:46 PM, Dr Andrew John Hughes
> <ahughes@...> wrote:
>> Thanks for catching this bug.  I was going to approve it but you seem to have already committed
>> it -- why???
>
> I thought that'd be OK since the fix was pretty simple and I was able
> to work out why the regression was there. Would you prefer that I wait
> for an explicit ACKs in the future?
>

Please just make it clear what you're asking for.  If it's a fix
you're sure about, then just post to the patches
list with the FYI: subject prefix and commit straight away.  If not,
prefix the subject with RFC: and wait
for feedback.

With this one, it was unclear whether you wanted feedback or not and
the commit wasn't immediate,
but delayed by two days.

http://www.gnu.org/software/classpath/docs/cp-hacking.html#SEC11 might
be helpful.

>                        Pekka
>
>



--
Andrew :-)

Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

Support Free Java!
Contribute to GNU Classpath and the OpenJDK
http://www.gnu.org/software/classpath
http://openjdk.java.net

PGP Key: 94EFD9D8 (http://subkeys.pgp.net)
Fingerprint: F8EF F1EA 401E 2E60 15FA  7927 142C 2591 94EF D9D8


Re: [PATCH] Fix Use ArrayList.add() in StyleSheet.resolveStyle

by Pekka Enberg-4 :: Rate this Message:

| View Threaded | Show Only this Message

Hi Andrew,

On Tue, Mar 15, 2011 at 12:47 PM, Dr Andrew John Hughes
<ahughes@...> wrote:
> Please just make it clear what you're asking for.  If it's a fix
> you're sure about, then just post to the patches
> list with the FYI: subject prefix and commit straight away.  If not,
> prefix the subject with RFC: and wait
> for feedback.
>
> With this one, it was unclear whether you wanted feedback or not and
> the commit wasn't immediate, but delayed by two days.

Oh, sorry about that - I'm still in my 'other project' mindset. I was
thought the fix was obvious enough but delayed the commit by few days
so people could take a look at the patch to see if you wanted me to
fix it in some other way.

/me goes to re-read the GNU Classpath Hacker's Guide...

                        Pekka


Re: [PATCH] Fix Use ArrayList.add() in StyleSheet.resolveStyle

by Andrew Hughes-7 :: Rate this Message:

| View Threaded | Show Only this Message

On 12:55 Tue 15 Mar     , Pekka Enberg wrote:

> Hi Andrew,
>
> On Tue, Mar 15, 2011 at 12:47 PM, Dr Andrew John Hughes
> <ahughes@...> wrote:
> > Please just make it clear what you're asking for.  If it's a fix
> > you're sure about, then just post to the patches
> > list with the FYI: subject prefix and commit straight away.  If not,
> > prefix the subject with RFC: and wait
> > for feedback.
> >
> > With this one, it was unclear whether you wanted feedback or not and
> > the commit wasn't immediate, but delayed by two days.
>
> Oh, sorry about that - I'm still in my 'other project' mindset. I was
> thought the fix was obvious enough but delayed the commit by few days
> so people could take a look at the patch to see if you wanted me to
> fix it in some other way.
>

Yeah it's just a bit confusing for anyone (probably just me...)
watching that process as you didn't reply to say why you were
committing...

> /me goes to re-read the GNU Classpath Hacker's Guide...
>
>                         Pekka

--
Andrew :)

Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

Support Free Java!
Contribute to GNU Classpath and IcedTea
http://www.gnu.org/software/classpath
http://icedtea.classpath.org
PGP Key: F5862A37 (https://keys.indymedia.org/)
Fingerprint = EA30 D855 D50F 90CD F54D  0698 0713 C3ED F586 2A37