Re: svn commit: r1748015 - in /commons/proper/imaging/trunk/src: changes/changes.xml main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java

classic Classic list List threaded Threaded
12 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r1748015 - in /commons/proper/imaging/trunk/src: changes/changes.xml main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java

garydgregory
This looks like a candidate for the more efficient switch on string.

Gary

On Sun, Jun 12, 2016 at 7:47 AM, <[hidden email]> wrote:

> Author: britter
> Date: Sun Jun 12 14:47:11 2016
> New Revision: 1748015
>
> URL: http://svn.apache.org/viewvc?rev=1748015&view=rev
> Log:
> IMAGING-178: PnmImageParser does not check the validity of input PAM
> header. Thanks to emopers. This also fixes #20 from github.
>
> Modified:
>     commons/proper/imaging/trunk/src/changes/changes.xml
>
> commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
>
> commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
>
> Modified: commons/proper/imaging/trunk/src/changes/changes.xml
> URL:
> http://svn.apache.org/viewvc/commons/proper/imaging/trunk/src/changes/changes.xml?rev=1748015&r1=1748014&r2=1748015&view=diff
>
> ==============================================================================
> --- commons/proper/imaging/trunk/src/changes/changes.xml (original)
> +++ commons/proper/imaging/trunk/src/changes/changes.xml Sun Jun 12
> 14:47:11 2016
> @@ -46,6 +46,9 @@ The <action> type attribute can be add,u
>    <body>
>
>      <release version="1.0" date="TBA" description="First major release">
> +      <action issue="IMAGING-178" dev="britter" type="fix"
> due-to="emopers">
> +        PnmImageParser does not check the validity of input PAM header.
> +      </action>
>        <action issue="IMAGING-184" dev="ggregory" type="update">
>          Update platform from Java 5 to 7
>        </action>
>
> Modified:
> commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> URL:
> http://svn.apache.org/viewvc/commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java?rev=1748015&r1=1748014&r2=1748015&view=diff
>
> ==============================================================================
> ---
> commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> (original)
> +++
> commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> Sun Jun 12 14:47:11 2016
> @@ -157,18 +157,28 @@ public class PnmImageParser extends Imag
>                  final String type = tokenizer.nextToken();
>                  if ("WIDTH".equals(type)) {
>                      seenWidth = true;
> +                    if(!tokenizer.hasMoreTokens())
> +                        throw new ImageReadException("PAM header has no
> WIDTH value");
>                      width = Integer.parseInt(tokenizer.nextToken());
>                  } else if ("HEIGHT".equals(type)) {
>                      seenHeight = true;
> +                    if(!tokenizer.hasMoreTokens())
> +                        throw new ImageReadException("PAM header has no
> HEIGHT value");
>                      height = Integer.parseInt(tokenizer.nextToken());
>                  } else if ("DEPTH".equals(type)) {
>                      seenDepth = true;
> +                    if(!tokenizer.hasMoreTokens())
> +                        throw new ImageReadException("PAM header has no
> DEPTH value");
>                      depth = Integer.parseInt(tokenizer.nextToken());
>                  } else if ("MAXVAL".equals(type)) {
>                      seenMaxVal = true;
> +                    if(!tokenizer.hasMoreTokens())
> +                        throw new ImageReadException("PAM header has no
> MAXVAL value");
>                      maxVal = Integer.parseInt(tokenizer.nextToken());
>                  } else if ("TUPLTYPE".equals(type)) {
>                      seenTupleType = true;
> +                    if(!tokenizer.hasMoreTokens())
> +                        throw new ImageReadException("PAM header has no
> TUPLTYPE value");
>                      tupleType.append(tokenizer.nextToken());
>                  } else if ("ENDHDR".equals(type)) {
>                      break;
>
> Modified:
> commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> URL:
> http://svn.apache.org/viewvc/commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java?rev=1748015&r1=1748014&r2=1748015&view=diff
>
> ==============================================================================
> ---
> commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> (original)
> +++
> commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> Sun Jun 12 14:47:11 2016
> @@ -46,4 +46,12 @@ public class PnmImageParserTest {
>      PnmImageParser underTest = new PnmImageParser();
>      underTest.getImageInfo(bytes, params);
>    }
> +
> +  @Test(expected = ImageReadException.class)
> +  public void testGetImageInfo_missingWidthValue() throws
> ImageReadException, IOException {
> +    byte[] bytes = "P7\nWIDTH \n".getBytes(US_ASCII);
> +    Map<String, Object> params = Collections.emptyMap();
> +    PnmImageParser underTest = new PnmImageParser();
> +    underTest.getImageInfo(bytes, params);
> +  }
>  }
>
>
>


--
E-Mail: [hidden email] | [hidden email]
Java Persistence with Hibernate, Second Edition
<http://www.manning.com/bauer3/>
JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
Spring Batch in Action <http://www.manning.com/templier/>
Blog: http://garygregory.wordpress.com
Home: http://garygregory.com/
Tweet! http://twitter.com/GaryGregory
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r1748015 - in /commons/proper/imaging/trunk/src: changes/changes.xml main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java

Benedikt Ritter-4
Yes, the if-else could also be implemented using a switch statement. But is
that really more efficient?

Benedikt

Gary Gregory <[hidden email]> schrieb am So., 12. Juni 2016 um
22:05:

> This looks like a candidate for the more efficient switch on string.
>
> Gary
>
> On Sun, Jun 12, 2016 at 7:47 AM, <[hidden email]> wrote:
>
> > Author: britter
> > Date: Sun Jun 12 14:47:11 2016
> > New Revision: 1748015
> >
> > URL: http://svn.apache.org/viewvc?rev=1748015&view=rev
> > Log:
> > IMAGING-178: PnmImageParser does not check the validity of input PAM
> > header. Thanks to emopers. This also fixes #20 from github.
> >
> > Modified:
> >     commons/proper/imaging/trunk/src/changes/changes.xml
> >
> >
> commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> >
> >
> commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> >
> > Modified: commons/proper/imaging/trunk/src/changes/changes.xml
> > URL:
> >
> http://svn.apache.org/viewvc/commons/proper/imaging/trunk/src/changes/changes.xml?rev=1748015&r1=1748014&r2=1748015&view=diff
> >
> >
> ==============================================================================
> > --- commons/proper/imaging/trunk/src/changes/changes.xml (original)
> > +++ commons/proper/imaging/trunk/src/changes/changes.xml Sun Jun 12
> > 14:47:11 2016
> > @@ -46,6 +46,9 @@ The <action> type attribute can be add,u
> >    <body>
> >
> >      <release version="1.0" date="TBA" description="First major release">
> > +      <action issue="IMAGING-178" dev="britter" type="fix"
> > due-to="emopers">
> > +        PnmImageParser does not check the validity of input PAM header.
> > +      </action>
> >        <action issue="IMAGING-184" dev="ggregory" type="update">
> >          Update platform from Java 5 to 7
> >        </action>
> >
> > Modified:
> >
> commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > URL:
> >
> http://svn.apache.org/viewvc/commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java?rev=1748015&r1=1748014&r2=1748015&view=diff
> >
> >
> ==============================================================================
> > ---
> >
> commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > (original)
> > +++
> >
> commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > Sun Jun 12 14:47:11 2016
> > @@ -157,18 +157,28 @@ public class PnmImageParser extends Imag
> >                  final String type = tokenizer.nextToken();
> >                  if ("WIDTH".equals(type)) {
> >                      seenWidth = true;
> > +                    if(!tokenizer.hasMoreTokens())
> > +                        throw new ImageReadException("PAM header has no
> > WIDTH value");
> >                      width = Integer.parseInt(tokenizer.nextToken());
> >                  } else if ("HEIGHT".equals(type)) {
> >                      seenHeight = true;
> > +                    if(!tokenizer.hasMoreTokens())
> > +                        throw new ImageReadException("PAM header has no
> > HEIGHT value");
> >                      height = Integer.parseInt(tokenizer.nextToken());
> >                  } else if ("DEPTH".equals(type)) {
> >                      seenDepth = true;
> > +                    if(!tokenizer.hasMoreTokens())
> > +                        throw new ImageReadException("PAM header has no
> > DEPTH value");
> >                      depth = Integer.parseInt(tokenizer.nextToken());
> >                  } else if ("MAXVAL".equals(type)) {
> >                      seenMaxVal = true;
> > +                    if(!tokenizer.hasMoreTokens())
> > +                        throw new ImageReadException("PAM header has no
> > MAXVAL value");
> >                      maxVal = Integer.parseInt(tokenizer.nextToken());
> >                  } else if ("TUPLTYPE".equals(type)) {
> >                      seenTupleType = true;
> > +                    if(!tokenizer.hasMoreTokens())
> > +                        throw new ImageReadException("PAM header has no
> > TUPLTYPE value");
> >                      tupleType.append(tokenizer.nextToken());
> >                  } else if ("ENDHDR".equals(type)) {
> >                      break;
> >
> > Modified:
> >
> commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> > URL:
> >
> http://svn.apache.org/viewvc/commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java?rev=1748015&r1=1748014&r2=1748015&view=diff
> >
> >
> ==============================================================================
> > ---
> >
> commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> > (original)
> > +++
> >
> commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> > Sun Jun 12 14:47:11 2016
> > @@ -46,4 +46,12 @@ public class PnmImageParserTest {
> >      PnmImageParser underTest = new PnmImageParser();
> >      underTest.getImageInfo(bytes, params);
> >    }
> > +
> > +  @Test(expected = ImageReadException.class)
> > +  public void testGetImageInfo_missingWidthValue() throws
> > ImageReadException, IOException {
> > +    byte[] bytes = "P7\nWIDTH \n".getBytes(US_ASCII);
> > +    Map<String, Object> params = Collections.emptyMap();
> > +    PnmImageParser underTest = new PnmImageParser();
> > +    underTest.getImageInfo(bytes, params);
> > +  }
> >  }
> >
> >
> >
>
>
> --
> E-Mail: [hidden email] | [hidden email]
> Java Persistence with Hibernate, Second Edition
> <http://www.manning.com/bauer3/>
> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
> Spring Batch in Action <http://www.manning.com/templier/>
> Blog: http://garygregory.wordpress.com
> Home: http://garygregory.com/
> Tweet! http://twitter.com/GaryGregory
>
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r1748015 - in /commons/proper/imaging/trunk/src: changes/changes.xml main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java

dbrosIus
In reply to this post by garydgregory
Yes. Only 1 hashCode/equals instead of N equals

-------- Original message --------
From: Benedikt Ritter <[hidden email]>
Date: 06/12/2016  4:37 PM  (GMT-05:00)
To: Commons Developers List <[hidden email]>
Subject: Re: svn commit: r1748015 - in /commons/proper/imaging/trunk/src: changes/changes.xml main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java

Yes, the if-else could also be implemented using a switch statement. But is
that really more efficient?

Benedikt

Gary Gregory <[hidden email]> schrieb am So., 12. Juni 2016 um
22:05:

> This looks like a candidate for the more efficient switch on string.
>
> Gary
>
> On Sun, Jun 12, 2016 at 7:47 AM, <[hidden email]> wrote:
>
> > Author: britter
> > Date: Sun Jun 12 14:47:11 2016
> > New Revision: 1748015
> >
> > URL: http://svn.apache.org/viewvc?rev=1748015&view=rev
> > Log:
> > IMAGING-178: PnmImageParser does not check the validity of input PAM
> > header. Thanks to emopers. This also fixes #20 from github.
> >
> > Modified:
> >     commons/proper/imaging/trunk/src/changes/changes.xml
> >
> >
> commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> >
> >
> commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> >
> > Modified: commons/proper/imaging/trunk/src/changes/changes.xml
> > URL:
> >
> http://svn.apache.org/viewvc/commons/proper/imaging/trunk/src/changes/changes.xml?rev=1748015&r1=1748014&r2=1748015&view=diff
> >
> >
> ==============================================================================
> > --- commons/proper/imaging/trunk/src/changes/changes.xml (original)
> > +++ commons/proper/imaging/trunk/src/changes/changes.xml Sun Jun 12
> > 14:47:11 2016
> > @@ -46,6 +46,9 @@ The <action> type attribute can be add,u
> >    <body>
> >
> >      <release version="1.0" date="TBA" description="First major release">
> > +      <action issue="IMAGING-178" dev="britter" type="fix"
> > due-to="emopers">
> > +        PnmImageParser does not check the validity of input PAM header.
> > +      </action>
> >        <action issue="IMAGING-184" dev="ggregory" type="update">
> >          Update platform from Java 5 to 7
> >        </action>
> >
> > Modified:
> >
> commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > URL:
> >
> http://svn.apache.org/viewvc/commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java?rev=1748015&r1=1748014&r2=1748015&view=diff
> >
> >
> ==============================================================================
> > ---
> >
> commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > (original)
> > +++
> >
> commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > Sun Jun 12 14:47:11 2016
> > @@ -157,18 +157,28 @@ public class PnmImageParser extends Imag
> >                  final String type = tokenizer.nextToken();
> >                  if ("WIDTH".equals(type)) {
> >                      seenWidth = true;
> > +                    if(!tokenizer.hasMoreTokens())
> > +                        throw new ImageReadException("PAM header has no
> > WIDTH value");
> >                      width = Integer.parseInt(tokenizer.nextToken());
> >                  } else if ("HEIGHT".equals(type)) {
> >                      seenHeight = true;
> > +                    if(!tokenizer.hasMoreTokens())
> > +                        throw new ImageReadException("PAM header has no
> > HEIGHT value");
> >                      height = Integer.parseInt(tokenizer.nextToken());
> >                  } else if ("DEPTH".equals(type)) {
> >                      seenDepth = true;
> > +                    if(!tokenizer.hasMoreTokens())
> > +                        throw new ImageReadException("PAM header has no
> > DEPTH value");
> >                      depth = Integer.parseInt(tokenizer.nextToken());
> >                  } else if ("MAXVAL".equals(type)) {
> >                      seenMaxVal = true;
> > +                    if(!tokenizer.hasMoreTokens())
> > +                        throw new ImageReadException("PAM header has no
> > MAXVAL value");
> >                      maxVal = Integer.parseInt(tokenizer.nextToken());
> >                  } else if ("TUPLTYPE".equals(type)) {
> >                      seenTupleType = true;
> > +                    if(!tokenizer.hasMoreTokens())
> > +                        throw new ImageReadException("PAM header has no
> > TUPLTYPE value");
> >                      tupleType.append(tokenizer.nextToken());
> >                  } else if ("ENDHDR".equals(type)) {
> >                      break;
> >
> > Modified:
> >
> commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> > URL:
> >
> http://svn.apache.org/viewvc/commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java?rev=1748015&r1=1748014&r2=1748015&view=diff
> >
> >
> ==============================================================================
> > ---
> >
> commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> > (original)
> > +++
> >
> commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> > Sun Jun 12 14:47:11 2016
> > @@ -46,4 +46,12 @@ public class PnmImageParserTest {
> >      PnmImageParser underTest = new PnmImageParser();
> >      underTest.getImageInfo(bytes, params);
> >    }
> > +
> > +  @Test(expected = ImageReadException.class)
> > +  public void testGetImageInfo_missingWidthValue() throws
> > ImageReadException, IOException {
> > +    byte[] bytes = "P7\nWIDTH \n".getBytes(US_ASCII);
> > +    Map<String, Object> params = Collections.emptyMap();
> > +    PnmImageParser underTest = new PnmImageParser();
> > +    underTest.getImageInfo(bytes, params);
> > +  }
> >  }
> >
> >
> >
>
>
> --
> E-Mail: [hidden email] | [hidden email]
> Java Persistence with Hibernate, Second Edition
> <http://www.manning.com/bauer3/>
> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
> Spring Batch in Action <http://www.manning.com/templier/>
> Blog: http://garygregory.wordpress.com
> Home: http://garygregory.com/
> Tweet! http://twitter.com/GaryGregory
>
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r1748015 - in /commons/proper/imaging/trunk/src: changes/changes.xml main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java

James Carman
In reply to this post by Benedikt Ritter-4
Doesn't it all compiled down to the same thing? It gets optimized at
runtime anyway. I would go for the more readable option, unless there are
extremely compelling performance numbers.

On Sun, Jun 12, 2016 at 4:38 PM Benedikt Ritter <[hidden email]> wrote:

> Yes, the if-else could also be implemented using a switch statement. But is
> that really more efficient?
>
> Benedikt
>
> Gary Gregory <[hidden email]> schrieb am So., 12. Juni 2016 um
> 22:05:
>
> > This looks like a candidate for the more efficient switch on string.
> >
> > Gary
> >
> > On Sun, Jun 12, 2016 at 7:47 AM, <[hidden email]> wrote:
> >
> > > Author: britter
> > > Date: Sun Jun 12 14:47:11 2016
> > > New Revision: 1748015
> > >
> > > URL: http://svn.apache.org/viewvc?rev=1748015&view=rev
> > > Log:
> > > IMAGING-178: PnmImageParser does not check the validity of input PAM
> > > header. Thanks to emopers. This also fixes #20 from github.
> > >
> > > Modified:
> > >     commons/proper/imaging/trunk/src/changes/changes.xml
> > >
> > >
> >
> commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > >
> > >
> >
> commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> > >
> > > Modified: commons/proper/imaging/trunk/src/changes/changes.xml
> > > URL:
> > >
> >
> http://svn.apache.org/viewvc/commons/proper/imaging/trunk/src/changes/changes.xml?rev=1748015&r1=1748014&r2=1748015&view=diff
> > >
> > >
> >
> ==============================================================================
> > > --- commons/proper/imaging/trunk/src/changes/changes.xml (original)
> > > +++ commons/proper/imaging/trunk/src/changes/changes.xml Sun Jun 12
> > > 14:47:11 2016
> > > @@ -46,6 +46,9 @@ The <action> type attribute can be add,u
> > >    <body>
> > >
> > >      <release version="1.0" date="TBA" description="First major
> release">
> > > +      <action issue="IMAGING-178" dev="britter" type="fix"
> > > due-to="emopers">
> > > +        PnmImageParser does not check the validity of input PAM
> header.
> > > +      </action>
> > >        <action issue="IMAGING-184" dev="ggregory" type="update">
> > >          Update platform from Java 5 to 7
> > >        </action>
> > >
> > > Modified:
> > >
> >
> commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > > URL:
> > >
> >
> http://svn.apache.org/viewvc/commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java?rev=1748015&r1=1748014&r2=1748015&view=diff
> > >
> > >
> >
> ==============================================================================
> > > ---
> > >
> >
> commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > > (original)
> > > +++
> > >
> >
> commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > > Sun Jun 12 14:47:11 2016
> > > @@ -157,18 +157,28 @@ public class PnmImageParser extends Imag
> > >                  final String type = tokenizer.nextToken();
> > >                  if ("WIDTH".equals(type)) {
> > >                      seenWidth = true;
> > > +                    if(!tokenizer.hasMoreTokens())
> > > +                        throw new ImageReadException("PAM header has
> no
> > > WIDTH value");
> > >                      width = Integer.parseInt(tokenizer.nextToken());
> > >                  } else if ("HEIGHT".equals(type)) {
> > >                      seenHeight = true;
> > > +                    if(!tokenizer.hasMoreTokens())
> > > +                        throw new ImageReadException("PAM header has
> no
> > > HEIGHT value");
> > >                      height = Integer.parseInt(tokenizer.nextToken());
> > >                  } else if ("DEPTH".equals(type)) {
> > >                      seenDepth = true;
> > > +                    if(!tokenizer.hasMoreTokens())
> > > +                        throw new ImageReadException("PAM header has
> no
> > > DEPTH value");
> > >                      depth = Integer.parseInt(tokenizer.nextToken());
> > >                  } else if ("MAXVAL".equals(type)) {
> > >                      seenMaxVal = true;
> > > +                    if(!tokenizer.hasMoreTokens())
> > > +                        throw new ImageReadException("PAM header has
> no
> > > MAXVAL value");
> > >                      maxVal = Integer.parseInt(tokenizer.nextToken());
> > >                  } else if ("TUPLTYPE".equals(type)) {
> > >                      seenTupleType = true;
> > > +                    if(!tokenizer.hasMoreTokens())
> > > +                        throw new ImageReadException("PAM header has
> no
> > > TUPLTYPE value");
> > >                      tupleType.append(tokenizer.nextToken());
> > >                  } else if ("ENDHDR".equals(type)) {
> > >                      break;
> > >
> > > Modified:
> > >
> >
> commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> > > URL:
> > >
> >
> http://svn.apache.org/viewvc/commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java?rev=1748015&r1=1748014&r2=1748015&view=diff
> > >
> > >
> >
> ==============================================================================
> > > ---
> > >
> >
> commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> > > (original)
> > > +++
> > >
> >
> commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> > > Sun Jun 12 14:47:11 2016
> > > @@ -46,4 +46,12 @@ public class PnmImageParserTest {
> > >      PnmImageParser underTest = new PnmImageParser();
> > >      underTest.getImageInfo(bytes, params);
> > >    }
> > > +
> > > +  @Test(expected = ImageReadException.class)
> > > +  public void testGetImageInfo_missingWidthValue() throws
> > > ImageReadException, IOException {
> > > +    byte[] bytes = "P7\nWIDTH \n".getBytes(US_ASCII);
> > > +    Map<String, Object> params = Collections.emptyMap();
> > > +    PnmImageParser underTest = new PnmImageParser();
> > > +    underTest.getImageInfo(bytes, params);
> > > +  }
> > >  }
> > >
> > >
> > >
> >
> >
> > --
> > E-Mail: [hidden email] | [hidden email]
> > Java Persistence with Hibernate, Second Edition
> > <http://www.manning.com/bauer3/>
> > JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
> > Spring Batch in Action <http://www.manning.com/templier/>
> > Blog: http://garygregory.wordpress.com
> > Home: http://garygregory.com/
> > Tweet! http://twitter.com/GaryGregory
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r1748015 - in /commons/proper/imaging/trunk/src: changes/changes.xml main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java

dbrosIus
In reply to this post by garydgregory
Interesting.  Source? 

-------- Original message --------
From: James Carman <[hidden email]>
Date: 06/12/2016  5:12 PM  (GMT-05:00)
To: Commons Developers List <[hidden email]>
Subject: Re: svn commit: r1748015 - in /commons/proper/imaging/trunk/src: changes/changes.xml main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java

Doesn't it all compiled down to the same thing? It gets optimized at
runtime anyway. I would go for the more readable option, unless there are
extremely compelling performance numbers.

On Sun, Jun 12, 2016 at 4:38 PM Benedikt Ritter <[hidden email]> wrote:

> Yes, the if-else could also be implemented using a switch statement. But is
> that really more efficient?
>
> Benedikt
>
> Gary Gregory <[hidden email]> schrieb am So., 12. Juni 2016 um
> 22:05:
>
> > This looks like a candidate for the more efficient switch on string.
> >
> > Gary
> >
> > On Sun, Jun 12, 2016 at 7:47 AM, <[hidden email]> wrote:
> >
> > > Author: britter
> > > Date: Sun Jun 12 14:47:11 2016
> > > New Revision: 1748015
> > >
> > > URL: http://svn.apache.org/viewvc?rev=1748015&view=rev
> > > Log:
> > > IMAGING-178: PnmImageParser does not check the validity of input PAM
> > > header. Thanks to emopers. This also fixes #20 from github.
> > >
> > > Modified:
> > >     commons/proper/imaging/trunk/src/changes/changes.xml
> > >
> > >
> >
> commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > >
> > >
> >
> commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> > >
> > > Modified: commons/proper/imaging/trunk/src/changes/changes.xml
> > > URL:
> > >
> >
> http://svn.apache.org/viewvc/commons/proper/imaging/trunk/src/changes/changes.xml?rev=1748015&r1=1748014&r2=1748015&view=diff
> > >
> > >
> >
> ==============================================================================
> > > --- commons/proper/imaging/trunk/src/changes/changes.xml (original)
> > > +++ commons/proper/imaging/trunk/src/changes/changes.xml Sun Jun 12
> > > 14:47:11 2016
> > > @@ -46,6 +46,9 @@ The <action> type attribute can be add,u
> > >    <body>
> > >
> > >      <release version="1.0" date="TBA" description="First major
> release">
> > > +      <action issue="IMAGING-178" dev="britter" type="fix"
> > > due-to="emopers">
> > > +        PnmImageParser does not check the validity of input PAM
> header.
> > > +      </action>
> > >        <action issue="IMAGING-184" dev="ggregory" type="update">
> > >          Update platform from Java 5 to 7
> > >        </action>
> > >
> > > Modified:
> > >
> >
> commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > > URL:
> > >
> >
> http://svn.apache.org/viewvc/commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java?rev=1748015&r1=1748014&r2=1748015&view=diff
> > >
> > >
> >
> ==============================================================================
> > > ---
> > >
> >
> commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > > (original)
> > > +++
> > >
> >
> commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > > Sun Jun 12 14:47:11 2016
> > > @@ -157,18 +157,28 @@ public class PnmImageParser extends Imag
> > >                  final String type = tokenizer.nextToken();
> > >                  if ("WIDTH".equals(type)) {
> > >                      seenWidth = true;
> > > +                    if(!tokenizer.hasMoreTokens())
> > > +                        throw new ImageReadException("PAM header has
> no
> > > WIDTH value");
> > >                      width = Integer.parseInt(tokenizer.nextToken());
> > >                  } else if ("HEIGHT".equals(type)) {
> > >                      seenHeight = true;
> > > +                    if(!tokenizer.hasMoreTokens())
> > > +                        throw new ImageReadException("PAM header has
> no
> > > HEIGHT value");
> > >                      height = Integer.parseInt(tokenizer.nextToken());
> > >                  } else if ("DEPTH".equals(type)) {
> > >                      seenDepth = true;
> > > +                    if(!tokenizer.hasMoreTokens())
> > > +                        throw new ImageReadException("PAM header has
> no
> > > DEPTH value");
> > >                      depth = Integer.parseInt(tokenizer.nextToken());
> > >                  } else if ("MAXVAL".equals(type)) {
> > >                      seenMaxVal = true;
> > > +                    if(!tokenizer.hasMoreTokens())
> > > +                        throw new ImageReadException("PAM header has
> no
> > > MAXVAL value");
> > >                      maxVal = Integer.parseInt(tokenizer.nextToken());
> > >                  } else if ("TUPLTYPE".equals(type)) {
> > >                      seenTupleType = true;
> > > +                    if(!tokenizer.hasMoreTokens())
> > > +                        throw new ImageReadException("PAM header has
> no
> > > TUPLTYPE value");
> > >                      tupleType.append(tokenizer.nextToken());
> > >                  } else if ("ENDHDR".equals(type)) {
> > >                      break;
> > >
> > > Modified:
> > >
> >
> commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> > > URL:
> > >
> >
> http://svn.apache.org/viewvc/commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java?rev=1748015&r1=1748014&r2=1748015&view=diff
> > >
> > >
> >
> ==============================================================================
> > > ---
> > >
> >
> commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> > > (original)
> > > +++
> > >
> >
> commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> > > Sun Jun 12 14:47:11 2016
> > > @@ -46,4 +46,12 @@ public class PnmImageParserTest {
> > >      PnmImageParser underTest = new PnmImageParser();
> > >      underTest.getImageInfo(bytes, params);
> > >    }
> > > +
> > > +  @Test(expected = ImageReadException.class)
> > > +  public void testGetImageInfo_missingWidthValue() throws
> > > ImageReadException, IOException {
> > > +    byte[] bytes = "P7\nWIDTH \n".getBytes(US_ASCII);
> > > +    Map<String, Object> params = Collections.emptyMap();
> > > +    PnmImageParser underTest = new PnmImageParser();
> > > +    underTest.getImageInfo(bytes, params);
> > > +  }
> > >  }
> > >
> > >
> > >
> >
> >
> > --
> > E-Mail: [hidden email] | [hidden email]
> > Java Persistence with Hibernate, Second Edition
> > <http://www.manning.com/bauer3/>
> > JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
> > Spring Batch in Action <http://www.manning.com/templier/>
> > Blog: http://garygregory.wordpress.com
> > Home: http://garygregory.com/
> > Tweet! http://twitter.com/GaryGregory
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r1748015 - in /commons/proper/imaging/trunk/src: changes/changes.xml main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java

James Carman
I was referring to JIT.

On Sun, Jun 12, 2016 at 5:18 PM dbrosIus <[hidden email]> wrote:

> Interesting.  Source?
>
> -------- Original message --------
> From: James Carman <[hidden email]>
> Date: 06/12/2016  5:12 PM  (GMT-05:00)
> To: Commons Developers List <[hidden email]>
> Subject: Re: svn commit: r1748015 - in /commons/proper/imaging/trunk/src:
> changes/changes.xml
> main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
>
> Doesn't it all compiled down to the same thing? It gets optimized at
> runtime anyway. I would go for the more readable option, unless there are
> extremely compelling performance numbers.
>
> On Sun, Jun 12, 2016 at 4:38 PM Benedikt Ritter <[hidden email]>
> wrote:
>
> > Yes, the if-else could also be implemented using a switch statement. But
> is
> > that really more efficient?
> >
> > Benedikt
> >
> > Gary Gregory <[hidden email]> schrieb am So., 12. Juni 2016 um
> > 22:05:
> >
> > > This looks like a candidate for the more efficient switch on string.
> > >
> > > Gary
> > >
> > > On Sun, Jun 12, 2016 at 7:47 AM, <[hidden email]> wrote:
> > >
> > > > Author: britter
> > > > Date: Sun Jun 12 14:47:11 2016
> > > > New Revision: 1748015
> > > >
> > > > URL: http://svn.apache.org/viewvc?rev=1748015&view=rev
> > > > Log:
> > > > IMAGING-178: PnmImageParser does not check the validity of input PAM
> > > > header. Thanks to emopers. This also fixes #20 from github.
> > > >
> > > > Modified:
> > > >     commons/proper/imaging/trunk/src/changes/changes.xml
> > > >
> > > >
> > >
> >
> commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > > >
> > > >
> > >
> >
> commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> > > >
> > > > Modified: commons/proper/imaging/trunk/src/changes/changes.xml
> > > > URL:
> > > >
> > >
> >
> http://svn.apache.org/viewvc/commons/proper/imaging/trunk/src/changes/changes.xml?rev=1748015&r1=1748014&r2=1748015&view=diff
> > > >
> > > >
> > >
> >
> ==============================================================================
> > > > --- commons/proper/imaging/trunk/src/changes/changes.xml (original)
> > > > +++ commons/proper/imaging/trunk/src/changes/changes.xml Sun Jun 12
> > > > 14:47:11 2016
> > > > @@ -46,6 +46,9 @@ The <action> type attribute can be add,u
> > > >    <body>
> > > >
> > > >      <release version="1.0" date="TBA" description="First major
> > release">
> > > > +      <action issue="IMAGING-178" dev="britter" type="fix"
> > > > due-to="emopers">
> > > > +        PnmImageParser does not check the validity of input PAM
> > header.
> > > > +      </action>
> > > >        <action issue="IMAGING-184" dev="ggregory" type="update">
> > > >          Update platform from Java 5 to 7
> > > >        </action>
> > > >
> > > > Modified:
> > > >
> > >
> >
> commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > > > URL:
> > > >
> > >
> >
> http://svn.apache.org/viewvc/commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java?rev=1748015&r1=1748014&r2=1748015&view=diff
> > > >
> > > >
> > >
> >
> ==============================================================================
> > > > ---
> > > >
> > >
> >
> commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > > > (original)
> > > > +++
> > > >
> > >
> >
> commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > > > Sun Jun 12 14:47:11 2016
> > > > @@ -157,18 +157,28 @@ public class PnmImageParser extends Imag
> > > >                  final String type = tokenizer.nextToken();
> > > >                  if ("WIDTH".equals(type)) {
> > > >                      seenWidth = true;
> > > > +                    if(!tokenizer.hasMoreTokens())
> > > > +                        throw new ImageReadException("PAM header has
> > no
> > > > WIDTH value");
> > > >                      width = Integer.parseInt(tokenizer.nextToken());
> > > >                  } else if ("HEIGHT".equals(type)) {
> > > >                      seenHeight = true;
> > > > +                    if(!tokenizer.hasMoreTokens())
> > > > +                        throw new ImageReadException("PAM header has
> > no
> > > > HEIGHT value");
> > > >                      height =
> Integer.parseInt(tokenizer.nextToken());
> > > >                  } else if ("DEPTH".equals(type)) {
> > > >                      seenDepth = true;
> > > > +                    if(!tokenizer.hasMoreTokens())
> > > > +                        throw new ImageReadException("PAM header has
> > no
> > > > DEPTH value");
> > > >                      depth = Integer.parseInt(tokenizer.nextToken());
> > > >                  } else if ("MAXVAL".equals(type)) {
> > > >                      seenMaxVal = true;
> > > > +                    if(!tokenizer.hasMoreTokens())
> > > > +                        throw new ImageReadException("PAM header has
> > no
> > > > MAXVAL value");
> > > >                      maxVal =
> Integer.parseInt(tokenizer.nextToken());
> > > >                  } else if ("TUPLTYPE".equals(type)) {
> > > >                      seenTupleType = true;
> > > > +                    if(!tokenizer.hasMoreTokens())
> > > > +                        throw new ImageReadException("PAM header has
> > no
> > > > TUPLTYPE value");
> > > >                      tupleType.append(tokenizer.nextToken());
> > > >                  } else if ("ENDHDR".equals(type)) {
> > > >                      break;
> > > >
> > > > Modified:
> > > >
> > >
> >
> commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> > > > URL:
> > > >
> > >
> >
> http://svn.apache.org/viewvc/commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java?rev=1748015&r1=1748014&r2=1748015&view=diff
> > > >
> > > >
> > >
> >
> ==============================================================================
> > > > ---
> > > >
> > >
> >
> commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> > > > (original)
> > > > +++
> > > >
> > >
> >
> commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> > > > Sun Jun 12 14:47:11 2016
> > > > @@ -46,4 +46,12 @@ public class PnmImageParserTest {
> > > >      PnmImageParser underTest = new PnmImageParser();
> > > >      underTest.getImageInfo(bytes, params);
> > > >    }
> > > > +
> > > > +  @Test(expected = ImageReadException.class)
> > > > +  public void testGetImageInfo_missingWidthValue() throws
> > > > ImageReadException, IOException {
> > > > +    byte[] bytes = "P7\nWIDTH \n".getBytes(US_ASCII);
> > > > +    Map<String, Object> params = Collections.emptyMap();
> > > > +    PnmImageParser underTest = new PnmImageParser();
> > > > +    underTest.getImageInfo(bytes, params);
> > > > +  }
> > > >  }
> > > >
> > > >
> > > >
> > >
> > >
> > > --
> > > E-Mail: [hidden email] | [hidden email]
> > > Java Persistence with Hibernate, Second Edition
> > > <http://www.manning.com/bauer3/>
> > > JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
> > > Spring Batch in Action <http://www.manning.com/templier/>
> > > Blog: http://garygregory.wordpress.com
> > > Home: http://garygregory.com/
> > > Tweet! http://twitter.com/GaryGregory
> > >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r1748015 - in /commons/proper/imaging/trunk/src: changes/changes.xml main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java

garydgregory
In reply to this post by James Carman
On Sun, Jun 12, 2016 at 2:12 PM, James Carman <[hidden email]>
wrote:

> Doesn't it all compiled down to the same thing?


You're assuming all compilers from all vendors would be smart enough to
optimize a cascading if-else into a switch-on-string. I would not bet my
money on that. But I do know how a switch-on-string works, it's a look up
table, O(1), and also less-error prone IMO.

Gary


> It gets optimized at
> runtime anyway. I would go for the more readable option, unless there are
> extremely compelling performance numbers.
>
> On Sun, Jun 12, 2016 at 4:38 PM Benedikt Ritter <[hidden email]>
> wrote:
>
> > Yes, the if-else could also be implemented using a switch statement. But
> is
> > that really more efficient?
> >
> > Benedikt
> >
> > Gary Gregory <[hidden email]> schrieb am So., 12. Juni 2016 um
> > 22:05:
> >
> > > This looks like a candidate for the more efficient switch on string.
> > >
> > > Gary
> > >
> > > On Sun, Jun 12, 2016 at 7:47 AM, <[hidden email]> wrote:
> > >
> > > > Author: britter
> > > > Date: Sun Jun 12 14:47:11 2016
> > > > New Revision: 1748015
> > > >
> > > > URL: http://svn.apache.org/viewvc?rev=1748015&view=rev
> > > > Log:
> > > > IMAGING-178: PnmImageParser does not check the validity of input PAM
> > > > header. Thanks to emopers. This also fixes #20 from github.
> > > >
> > > > Modified:
> > > >     commons/proper/imaging/trunk/src/changes/changes.xml
> > > >
> > > >
> > >
> >
> commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > > >
> > > >
> > >
> >
> commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> > > >
> > > > Modified: commons/proper/imaging/trunk/src/changes/changes.xml
> > > > URL:
> > > >
> > >
> >
> http://svn.apache.org/viewvc/commons/proper/imaging/trunk/src/changes/changes.xml?rev=1748015&r1=1748014&r2=1748015&view=diff
> > > >
> > > >
> > >
> >
> ==============================================================================
> > > > --- commons/proper/imaging/trunk/src/changes/changes.xml (original)
> > > > +++ commons/proper/imaging/trunk/src/changes/changes.xml Sun Jun 12
> > > > 14:47:11 2016
> > > > @@ -46,6 +46,9 @@ The <action> type attribute can be add,u
> > > >    <body>
> > > >
> > > >      <release version="1.0" date="TBA" description="First major
> > release">
> > > > +      <action issue="IMAGING-178" dev="britter" type="fix"
> > > > due-to="emopers">
> > > > +        PnmImageParser does not check the validity of input PAM
> > header.
> > > > +      </action>
> > > >        <action issue="IMAGING-184" dev="ggregory" type="update">
> > > >          Update platform from Java 5 to 7
> > > >        </action>
> > > >
> > > > Modified:
> > > >
> > >
> >
> commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > > > URL:
> > > >
> > >
> >
> http://svn.apache.org/viewvc/commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java?rev=1748015&r1=1748014&r2=1748015&view=diff
> > > >
> > > >
> > >
> >
> ==============================================================================
> > > > ---
> > > >
> > >
> >
> commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > > > (original)
> > > > +++
> > > >
> > >
> >
> commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > > > Sun Jun 12 14:47:11 2016
> > > > @@ -157,18 +157,28 @@ public class PnmImageParser extends Imag
> > > >                  final String type = tokenizer.nextToken();
> > > >                  if ("WIDTH".equals(type)) {
> > > >                      seenWidth = true;
> > > > +                    if(!tokenizer.hasMoreTokens())
> > > > +                        throw new ImageReadException("PAM header has
> > no
> > > > WIDTH value");
> > > >                      width = Integer.parseInt(tokenizer.nextToken());
> > > >                  } else if ("HEIGHT".equals(type)) {
> > > >                      seenHeight = true;
> > > > +                    if(!tokenizer.hasMoreTokens())
> > > > +                        throw new ImageReadException("PAM header has
> > no
> > > > HEIGHT value");
> > > >                      height =
> Integer.parseInt(tokenizer.nextToken());
> > > >                  } else if ("DEPTH".equals(type)) {
> > > >                      seenDepth = true;
> > > > +                    if(!tokenizer.hasMoreTokens())
> > > > +                        throw new ImageReadException("PAM header has
> > no
> > > > DEPTH value");
> > > >                      depth = Integer.parseInt(tokenizer.nextToken());
> > > >                  } else if ("MAXVAL".equals(type)) {
> > > >                      seenMaxVal = true;
> > > > +                    if(!tokenizer.hasMoreTokens())
> > > > +                        throw new ImageReadException("PAM header has
> > no
> > > > MAXVAL value");
> > > >                      maxVal =
> Integer.parseInt(tokenizer.nextToken());
> > > >                  } else if ("TUPLTYPE".equals(type)) {
> > > >                      seenTupleType = true;
> > > > +                    if(!tokenizer.hasMoreTokens())
> > > > +                        throw new ImageReadException("PAM header has
> > no
> > > > TUPLTYPE value");
> > > >                      tupleType.append(tokenizer.nextToken());
> > > >                  } else if ("ENDHDR".equals(type)) {
> > > >                      break;
> > > >
> > > > Modified:
> > > >
> > >
> >
> commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> > > > URL:
> > > >
> > >
> >
> http://svn.apache.org/viewvc/commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java?rev=1748015&r1=1748014&r2=1748015&view=diff
> > > >
> > > >
> > >
> >
> ==============================================================================
> > > > ---
> > > >
> > >
> >
> commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> > > > (original)
> > > > +++
> > > >
> > >
> >
> commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> > > > Sun Jun 12 14:47:11 2016
> > > > @@ -46,4 +46,12 @@ public class PnmImageParserTest {
> > > >      PnmImageParser underTest = new PnmImageParser();
> > > >      underTest.getImageInfo(bytes, params);
> > > >    }
> > > > +
> > > > +  @Test(expected = ImageReadException.class)
> > > > +  public void testGetImageInfo_missingWidthValue() throws
> > > > ImageReadException, IOException {
> > > > +    byte[] bytes = "P7\nWIDTH \n".getBytes(US_ASCII);
> > > > +    Map<String, Object> params = Collections.emptyMap();
> > > > +    PnmImageParser underTest = new PnmImageParser();
> > > > +    underTest.getImageInfo(bytes, params);
> > > > +  }
> > > >  }
> > > >
> > > >
> > > >
> > >
> > >
> > > --
> > > E-Mail: [hidden email] | [hidden email]
> > > Java Persistence with Hibernate, Second Edition
> > > <http://www.manning.com/bauer3/>
> > > JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
> > > Spring Batch in Action <http://www.manning.com/templier/>
> > > Blog: http://garygregory.wordpress.com
> > > Home: http://garygregory.com/
> > > Tweet! http://twitter.com/GaryGregory
> > >
> >
>



--
E-Mail: [hidden email] | [hidden email]
Java Persistence with Hibernate, Second Edition
<http://www.manning.com/bauer3/>
JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
Spring Batch in Action <http://www.manning.com/templier/>
Blog: http://garygregory.wordpress.com
Home: http://garygregory.com/
Tweet! http://twitter.com/GaryGregory
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r1748015 - in /commons/proper/imaging/trunk/src: changes/changes.xml main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java

dbrosIus
In reply to this post by garydgregory
Sure I got that.  I thought you had a source that explains an if/else chain getting jitted down to a simple hash jumptable

-------- Original message --------
From: James Carman <[hidden email]>
Date: 06/12/2016  5:19 PM  (GMT-05:00)
To: Commons Developers List <[hidden email]>
Subject: Re: svn commit: r1748015 - in /commons/proper/imaging/trunk/src: changes/changes.xml main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java

I was referring to JIT.

On Sun, Jun 12, 2016 at 5:18 PM dbrosIus <[hidden email]> wrote:

> Interesting.  Source?
>
> -------- Original message --------
> From: James Carman <[hidden email]>
> Date: 06/12/2016  5:12 PM  (GMT-05:00)
> To: Commons Developers List <[hidden email]>
> Subject: Re: svn commit: r1748015 - in /commons/proper/imaging/trunk/src:
> changes/changes.xml
> main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
>
> Doesn't it all compiled down to the same thing? It gets optimized at
> runtime anyway. I would go for the more readable option, unless there are
> extremely compelling performance numbers.
>
> On Sun, Jun 12, 2016 at 4:38 PM Benedikt Ritter <[hidden email]>
> wrote:
>
> > Yes, the if-else could also be implemented using a switch statement. But
> is
> > that really more efficient?
> >
> > Benedikt
> >
> > Gary Gregory <[hidden email]> schrieb am So., 12. Juni 2016 um
> > 22:05:
> >
> > > This looks like a candidate for the more efficient switch on string.
> > >
> > > Gary
> > >
> > > On Sun, Jun 12, 2016 at 7:47 AM, <[hidden email]> wrote:
> > >
> > > > Author: britter
> > > > Date: Sun Jun 12 14:47:11 2016
> > > > New Revision: 1748015
> > > >
> > > > URL: http://svn.apache.org/viewvc?rev=1748015&view=rev
> > > > Log:
> > > > IMAGING-178: PnmImageParser does not check the validity of input PAM
> > > > header. Thanks to emopers. This also fixes #20 from github.
> > > >
> > > > Modified:
> > > >     commons/proper/imaging/trunk/src/changes/changes.xml
> > > >
> > > >
> > >
> >
> commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > > >
> > > >
> > >
> >
> commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> > > >
> > > > Modified: commons/proper/imaging/trunk/src/changes/changes.xml
> > > > URL:
> > > >
> > >
> >
> http://svn.apache.org/viewvc/commons/proper/imaging/trunk/src/changes/changes.xml?rev=1748015&r1=1748014&r2=1748015&view=diff
> > > >
> > > >
> > >
> >
> ==============================================================================
> > > > --- commons/proper/imaging/trunk/src/changes/changes.xml (original)
> > > > +++ commons/proper/imaging/trunk/src/changes/changes.xml Sun Jun 12
> > > > 14:47:11 2016
> > > > @@ -46,6 +46,9 @@ The <action> type attribute can be add,u
> > > >    <body>
> > > >
> > > >      <release version="1.0" date="TBA" description="First major
> > release">
> > > > +      <action issue="IMAGING-178" dev="britter" type="fix"
> > > > due-to="emopers">
> > > > +        PnmImageParser does not check the validity of input PAM
> > header.
> > > > +      </action>
> > > >        <action issue="IMAGING-184" dev="ggregory" type="update">
> > > >          Update platform from Java 5 to 7
> > > >        </action>
> > > >
> > > > Modified:
> > > >
> > >
> >
> commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > > > URL:
> > > >
> > >
> >
> http://svn.apache.org/viewvc/commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java?rev=1748015&r1=1748014&r2=1748015&view=diff
> > > >
> > > >
> > >
> >
> ==============================================================================
> > > > ---
> > > >
> > >
> >
> commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > > > (original)
> > > > +++
> > > >
> > >
> >
> commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > > > Sun Jun 12 14:47:11 2016
> > > > @@ -157,18 +157,28 @@ public class PnmImageParser extends Imag
> > > >                  final String type = tokenizer.nextToken();
> > > >                  if ("WIDTH".equals(type)) {
> > > >                      seenWidth = true;
> > > > +                    if(!tokenizer.hasMoreTokens())
> > > > +                        throw new ImageReadException("PAM header has
> > no
> > > > WIDTH value");
> > > >                      width = Integer.parseInt(tokenizer.nextToken());
> > > >                  } else if ("HEIGHT".equals(type)) {
> > > >                      seenHeight = true;
> > > > +                    if(!tokenizer.hasMoreTokens())
> > > > +                        throw new ImageReadException("PAM header has
> > no
> > > > HEIGHT value");
> > > >                      height =
> Integer.parseInt(tokenizer.nextToken());
> > > >                  } else if ("DEPTH".equals(type)) {
> > > >                      seenDepth = true;
> > > > +                    if(!tokenizer.hasMoreTokens())
> > > > +                        throw new ImageReadException("PAM header has
> > no
> > > > DEPTH value");
> > > >                      depth = Integer.parseInt(tokenizer.nextToken());
> > > >                  } else if ("MAXVAL".equals(type)) {
> > > >                      seenMaxVal = true;
> > > > +                    if(!tokenizer.hasMoreTokens())
> > > > +                        throw new ImageReadException("PAM header has
> > no
> > > > MAXVAL value");
> > > >                      maxVal =
> Integer.parseInt(tokenizer.nextToken());
> > > >                  } else if ("TUPLTYPE".equals(type)) {
> > > >                      seenTupleType = true;
> > > > +                    if(!tokenizer.hasMoreTokens())
> > > > +                        throw new ImageReadException("PAM header has
> > no
> > > > TUPLTYPE value");
> > > >                      tupleType.append(tokenizer.nextToken());
> > > >                  } else if ("ENDHDR".equals(type)) {
> > > >                      break;
> > > >
> > > > Modified:
> > > >
> > >
> >
> commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> > > > URL:
> > > >
> > >
> >
> http://svn.apache.org/viewvc/commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java?rev=1748015&r1=1748014&r2=1748015&view=diff
> > > >
> > > >
> > >
> >
> ==============================================================================
> > > > ---
> > > >
> > >
> >
> commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> > > > (original)
> > > > +++
> > > >
> > >
> >
> commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> > > > Sun Jun 12 14:47:11 2016
> > > > @@ -46,4 +46,12 @@ public class PnmImageParserTest {
> > > >      PnmImageParser underTest = new PnmImageParser();
> > > >      underTest.getImageInfo(bytes, params);
> > > >    }
> > > > +
> > > > +  @Test(expected = ImageReadException.class)
> > > > +  public void testGetImageInfo_missingWidthValue() throws
> > > > ImageReadException, IOException {
> > > > +    byte[] bytes = "P7\nWIDTH \n".getBytes(US_ASCII);
> > > > +    Map<String, Object> params = Collections.emptyMap();
> > > > +    PnmImageParser underTest = new PnmImageParser();
> > > > +    underTest.getImageInfo(bytes, params);
> > > > +  }
> > > >  }
> > > >
> > > >
> > > >
> > >
> > >
> > > --
> > > E-Mail: [hidden email] | [hidden email]
> > > Java Persistence with Hibernate, Second Edition
> > > <http://www.manning.com/bauer3/>
> > > JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
> > > Spring Batch in Action <http://www.manning.com/templier/>
> > > Blog: http://garygregory.wordpress.com
> > > Home: http://garygregory.com/
> > > Tweet! http://twitter.com/GaryGregory
> > >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r1748015 - in /commons/proper/imaging/trunk/src: changes/changes.xml main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java

James Carman
I don't off the top of my head. All I have is anecdotal stuff in my brain,
but then again I can't remember what I had for lunch yesterday either. :)

On Sun, Jun 12, 2016 at 5:42 PM dbrosIus <[hidden email]> wrote:

> Sure I got that.  I thought you had a source that explains an if/else
> chain getting jitted down to a simple hash jumptable
>
> -------- Original message --------
> From: James Carman <[hidden email]>
> Date: 06/12/2016  5:19 PM  (GMT-05:00)
> To: Commons Developers List <[hidden email]>
> Subject: Re: svn commit: r1748015 - in /commons/proper/imaging/trunk/src:
> changes/changes.xml
> main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
>
> I was referring to JIT.
>
> On Sun, Jun 12, 2016 at 5:18 PM dbrosIus <[hidden email]>
> wrote:
>
> > Interesting.  Source?
> >
> > -------- Original message --------
> > From: James Carman <[hidden email]>
> > Date: 06/12/2016  5:12 PM  (GMT-05:00)
> > To: Commons Developers List <[hidden email]>
> > Subject: Re: svn commit: r1748015 - in /commons/proper/imaging/trunk/src:
> > changes/changes.xml
> > main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> >
> > Doesn't it all compiled down to the same thing? It gets optimized at
> > runtime anyway. I would go for the more readable option, unless there are
> > extremely compelling performance numbers.
> >
> > On Sun, Jun 12, 2016 at 4:38 PM Benedikt Ritter <[hidden email]>
> > wrote:
> >
> > > Yes, the if-else could also be implemented using a switch statement.
> But
> > is
> > > that really more efficient?
> > >
> > > Benedikt
> > >
> > > Gary Gregory <[hidden email]> schrieb am So., 12. Juni 2016 um
> > > 22:05:
> > >
> > > > This looks like a candidate for the more efficient switch on string.
> > > >
> > > > Gary
> > > >
> > > > On Sun, Jun 12, 2016 at 7:47 AM, <[hidden email]> wrote:
> > > >
> > > > > Author: britter
> > > > > Date: Sun Jun 12 14:47:11 2016
> > > > > New Revision: 1748015
> > > > >
> > > > > URL: http://svn.apache.org/viewvc?rev=1748015&view=rev
> > > > > Log:
> > > > > IMAGING-178: PnmImageParser does not check the validity of input
> PAM
> > > > > header. Thanks to emopers. This also fixes #20 from github.
> > > > >
> > > > > Modified:
> > > > >     commons/proper/imaging/trunk/src/changes/changes.xml
> > > > >
> > > > >
> > > >
> > >
> >
> commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > > > >
> > > > >
> > > >
> > >
> >
> commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> > > > >
> > > > > Modified: commons/proper/imaging/trunk/src/changes/changes.xml
> > > > > URL:
> > > > >
> > > >
> > >
> >
> http://svn.apache.org/viewvc/commons/proper/imaging/trunk/src/changes/changes.xml?rev=1748015&r1=1748014&r2=1748015&view=diff
> > > > >
> > > > >
> > > >
> > >
> >
> ==============================================================================
> > > > > --- commons/proper/imaging/trunk/src/changes/changes.xml (original)
> > > > > +++ commons/proper/imaging/trunk/src/changes/changes.xml Sun Jun 12
> > > > > 14:47:11 2016
> > > > > @@ -46,6 +46,9 @@ The <action> type attribute can be add,u
> > > > >    <body>
> > > > >
> > > > >      <release version="1.0" date="TBA" description="First major
> > > release">
> > > > > +      <action issue="IMAGING-178" dev="britter" type="fix"
> > > > > due-to="emopers">
> > > > > +        PnmImageParser does not check the validity of input PAM
> > > header.
> > > > > +      </action>
> > > > >        <action issue="IMAGING-184" dev="ggregory" type="update">
> > > > >          Update platform from Java 5 to 7
> > > > >        </action>
> > > > >
> > > > > Modified:
> > > > >
> > > >
> > >
> >
> commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > > > > URL:
> > > > >
> > > >
> > >
> >
> http://svn.apache.org/viewvc/commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java?rev=1748015&r1=1748014&r2=1748015&view=diff
> > > > >
> > > > >
> > > >
> > >
> >
> ==============================================================================
> > > > > ---
> > > > >
> > > >
> > >
> >
> commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > > > > (original)
> > > > > +++
> > > > >
> > > >
> > >
> >
> commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > > > > Sun Jun 12 14:47:11 2016
> > > > > @@ -157,18 +157,28 @@ public class PnmImageParser extends Imag
> > > > >                  final String type = tokenizer.nextToken();
> > > > >                  if ("WIDTH".equals(type)) {
> > > > >                      seenWidth = true;
> > > > > +                    if(!tokenizer.hasMoreTokens())
> > > > > +                        throw new ImageReadException("PAM header
> has
> > > no
> > > > > WIDTH value");
> > > > >                      width =
> Integer.parseInt(tokenizer.nextToken());
> > > > >                  } else if ("HEIGHT".equals(type)) {
> > > > >                      seenHeight = true;
> > > > > +                    if(!tokenizer.hasMoreTokens())
> > > > > +                        throw new ImageReadException("PAM header
> has
> > > no
> > > > > HEIGHT value");
> > > > >                      height =
> > Integer.parseInt(tokenizer.nextToken());
> > > > >                  } else if ("DEPTH".equals(type)) {
> > > > >                      seenDepth = true;
> > > > > +                    if(!tokenizer.hasMoreTokens())
> > > > > +                        throw new ImageReadException("PAM header
> has
> > > no
> > > > > DEPTH value");
> > > > >                      depth =
> Integer.parseInt(tokenizer.nextToken());
> > > > >                  } else if ("MAXVAL".equals(type)) {
> > > > >                      seenMaxVal = true;
> > > > > +                    if(!tokenizer.hasMoreTokens())
> > > > > +                        throw new ImageReadException("PAM header
> has
> > > no
> > > > > MAXVAL value");
> > > > >                      maxVal =
> > Integer.parseInt(tokenizer.nextToken());
> > > > >                  } else if ("TUPLTYPE".equals(type)) {
> > > > >                      seenTupleType = true;
> > > > > +                    if(!tokenizer.hasMoreTokens())
> > > > > +                        throw new ImageReadException("PAM header
> has
> > > no
> > > > > TUPLTYPE value");
> > > > >                      tupleType.append(tokenizer.nextToken());
> > > > >                  } else if ("ENDHDR".equals(type)) {
> > > > >                      break;
> > > > >
> > > > > Modified:
> > > > >
> > > >
> > >
> >
> commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> > > > > URL:
> > > > >
> > > >
> > >
> >
> http://svn.apache.org/viewvc/commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java?rev=1748015&r1=1748014&r2=1748015&view=diff
> > > > >
> > > > >
> > > >
> > >
> >
> ==============================================================================
> > > > > ---
> > > > >
> > > >
> > >
> >
> commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> > > > > (original)
> > > > > +++
> > > > >
> > > >
> > >
> >
> commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> > > > > Sun Jun 12 14:47:11 2016
> > > > > @@ -46,4 +46,12 @@ public class PnmImageParserTest {
> > > > >      PnmImageParser underTest = new PnmImageParser();
> > > > >      underTest.getImageInfo(bytes, params);
> > > > >    }
> > > > > +
> > > > > +  @Test(expected = ImageReadException.class)
> > > > > +  public void testGetImageInfo_missingWidthValue() throws
> > > > > ImageReadException, IOException {
> > > > > +    byte[] bytes = "P7\nWIDTH \n".getBytes(US_ASCII);
> > > > > +    Map<String, Object> params = Collections.emptyMap();
> > > > > +    PnmImageParser underTest = new PnmImageParser();
> > > > > +    underTest.getImageInfo(bytes, params);
> > > > > +  }
> > > > >  }
> > > > >
> > > > >
> > > > >
> > > >
> > > >
> > > > --
> > > > E-Mail: [hidden email] | [hidden email]
> > > > Java Persistence with Hibernate, Second Edition
> > > > <http://www.manning.com/bauer3/>
> > > > JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
> > > > Spring Batch in Action <http://www.manning.com/templier/>
> > > > Blog: http://garygregory.wordpress.com
> > > > Home: http://garygregory.com/
> > > > Tweet! http://twitter.com/GaryGregory
> > > >
> > >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r1748015 - in /commons/proper/imaging/trunk/src: changes/changes.xml main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java

garydgregory
On Jun 12, 2016 2:45 PM, "James Carman" <[hidden email]> wrote:
>
> I don't off the top of my head. All I have is anecdotal stuff in my brain,
> but then again I can't remember what I had for lunch yesterday either. :)

How about breakfast this morning? ;-)

Gary
>
> On Sun, Jun 12, 2016 at 5:42 PM dbrosIus <[hidden email]>
wrote:
>
> > Sure I got that.  I thought you had a source that explains an if/else
> > chain getting jitted down to a simple hash jumptable
> >
> > -------- Original message --------
> > From: James Carman <[hidden email]>
> > Date: 06/12/2016  5:19 PM  (GMT-05:00)
> > To: Commons Developers List <[hidden email]>
> > Subject: Re: svn commit: r1748015 - in
/commons/proper/imaging/trunk/src:

> > changes/changes.xml
> > main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> >
> > I was referring to JIT.
> >
> > On Sun, Jun 12, 2016 at 5:18 PM dbrosIus <[hidden email]>
> > wrote:
> >
> > > Interesting.  Source?
> > >
> > > -------- Original message --------
> > > From: James Carman <[hidden email]>
> > > Date: 06/12/2016  5:12 PM  (GMT-05:00)
> > > To: Commons Developers List <[hidden email]>
> > > Subject: Re: svn commit: r1748015 - in
/commons/proper/imaging/trunk/src:
> > > changes/changes.xml
> > > main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > >
test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> > >
> > > Doesn't it all compiled down to the same thing? It gets optimized at
> > > runtime anyway. I would go for the more readable option, unless there
are

> > > extremely compelling performance numbers.
> > >
> > > On Sun, Jun 12, 2016 at 4:38 PM Benedikt Ritter <[hidden email]>
> > > wrote:
> > >
> > > > Yes, the if-else could also be implemented using a switch statement.
> > But
> > > is
> > > > that really more efficient?
> > > >
> > > > Benedikt
> > > >
> > > > Gary Gregory <[hidden email]> schrieb am So., 12. Juni 2016
um
> > > > 22:05:
> > > >
> > > > > This looks like a candidate for the more efficient switch on
string.

> > > > >
> > > > > Gary
> > > > >
> > > > > On Sun, Jun 12, 2016 at 7:47 AM, <[hidden email]> wrote:
> > > > >
> > > > > > Author: britter
> > > > > > Date: Sun Jun 12 14:47:11 2016
> > > > > > New Revision: 1748015
> > > > > >
> > > > > > URL: http://svn.apache.org/viewvc?rev=1748015&view=rev
> > > > > > Log:
> > > > > > IMAGING-178: PnmImageParser does not check the validity of input
> > PAM
> > > > > > header. Thanks to emopers. This also fixes #20 from github.
> > > > > >
> > > > > > Modified:
> > > > > >     commons/proper/imaging/trunk/src/changes/changes.xml
> > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> > > > > >
> > > > > > Modified: commons/proper/imaging/trunk/src/changes/changes.xml
> > > > > > URL:
> > > > > >
> > > > >
> > > >
> > >
> >
http://svn.apache.org/viewvc/commons/proper/imaging/trunk/src/changes/changes.xml?rev=1748015&r1=1748014&r2=1748015&view=diff
> > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
==============================================================================
> > > > > > --- commons/proper/imaging/trunk/src/changes/changes.xml
(original)
> > > > > > +++ commons/proper/imaging/trunk/src/changes/changes.xml Sun
Jun 12

> > > > > > 14:47:11 2016
> > > > > > @@ -46,6 +46,9 @@ The <action> type attribute can be add,u
> > > > > >    <body>
> > > > > >
> > > > > >      <release version="1.0" date="TBA" description="First major
> > > > release">
> > > > > > +      <action issue="IMAGING-178" dev="britter" type="fix"
> > > > > > due-to="emopers">
> > > > > > +        PnmImageParser does not check the validity of input PAM
> > > > header.
> > > > > > +      </action>
> > > > > >        <action issue="IMAGING-184" dev="ggregory" type="update">
> > > > > >          Update platform from Java 5 to 7
> > > > > >        </action>
> > > > > >
> > > > > > Modified:
> > > > > >
> > > > >
> > > >
> > >
> >
commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > > > > > URL:
> > > > > >
> > > > >
> > > >
> > >
> >
http://svn.apache.org/viewvc/commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java?rev=1748015&r1=1748014&r2=1748015&view=diff
> > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
==============================================================================
> > > > > > ---
> > > > > >
> > > > >
> > > >
> > >
> >
commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > > > > > (original)
> > > > > > +++
> > > > > >
> > > > >
> > > >
> > >
> >
commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > > > > > Sun Jun 12 14:47:11 2016
> > > > > > @@ -157,18 +157,28 @@ public class PnmImageParser extends Imag
> > > > > >                  final String type = tokenizer.nextToken();
> > > > > >                  if ("WIDTH".equals(type)) {
> > > > > >                      seenWidth = true;
> > > > > > +                    if(!tokenizer.hasMoreTokens())
> > > > > > +                        throw new ImageReadException("PAM
header
> > has
> > > > no
> > > > > > WIDTH value");
> > > > > >                      width =
> > Integer.parseInt(tokenizer.nextToken());
> > > > > >                  } else if ("HEIGHT".equals(type)) {
> > > > > >                      seenHeight = true;
> > > > > > +                    if(!tokenizer.hasMoreTokens())
> > > > > > +                        throw new ImageReadException("PAM
header
> > has
> > > > no
> > > > > > HEIGHT value");
> > > > > >                      height =
> > > Integer.parseInt(tokenizer.nextToken());
> > > > > >                  } else if ("DEPTH".equals(type)) {
> > > > > >                      seenDepth = true;
> > > > > > +                    if(!tokenizer.hasMoreTokens())
> > > > > > +                        throw new ImageReadException("PAM
header
> > has
> > > > no
> > > > > > DEPTH value");
> > > > > >                      depth =
> > Integer.parseInt(tokenizer.nextToken());
> > > > > >                  } else if ("MAXVAL".equals(type)) {
> > > > > >                      seenMaxVal = true;
> > > > > > +                    if(!tokenizer.hasMoreTokens())
> > > > > > +                        throw new ImageReadException("PAM
header
> > has
> > > > no
> > > > > > MAXVAL value");
> > > > > >                      maxVal =
> > > Integer.parseInt(tokenizer.nextToken());
> > > > > >                  } else if ("TUPLTYPE".equals(type)) {
> > > > > >                      seenTupleType = true;
> > > > > > +                    if(!tokenizer.hasMoreTokens())
> > > > > > +                        throw new ImageReadException("PAM
header

> > has
> > > > no
> > > > > > TUPLTYPE value");
> > > > > >                      tupleType.append(tokenizer.nextToken());
> > > > > >                  } else if ("ENDHDR".equals(type)) {
> > > > > >                      break;
> > > > > >
> > > > > > Modified:
> > > > > >
> > > > >
> > > >
> > >
> >
commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> > > > > > URL:
> > > > > >
> > > > >
> > > >
> > >
> >
http://svn.apache.org/viewvc/commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java?rev=1748015&r1=1748014&r2=1748015&view=diff
> > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
==============================================================================
> > > > > > ---
> > > > > >
> > > > >
> > > >
> > >
> >
commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> > > > > > (original)
> > > > > > +++
> > > > > >
> > > > >
> > > >
> > >
> >
commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java

> > > > > > Sun Jun 12 14:47:11 2016
> > > > > > @@ -46,4 +46,12 @@ public class PnmImageParserTest {
> > > > > >      PnmImageParser underTest = new PnmImageParser();
> > > > > >      underTest.getImageInfo(bytes, params);
> > > > > >    }
> > > > > > +
> > > > > > +  @Test(expected = ImageReadException.class)
> > > > > > +  public void testGetImageInfo_missingWidthValue() throws
> > > > > > ImageReadException, IOException {
> > > > > > +    byte[] bytes = "P7\nWIDTH \n".getBytes(US_ASCII);
> > > > > > +    Map<String, Object> params = Collections.emptyMap();
> > > > > > +    PnmImageParser underTest = new PnmImageParser();
> > > > > > +    underTest.getImageInfo(bytes, params);
> > > > > > +  }
> > > > > >  }
> > > > > >
> > > > > >
> > > > > >
> > > > >
> > > > >
> > > > > --
> > > > > E-Mail: [hidden email] | [hidden email]
> > > > > Java Persistence with Hibernate, Second Edition
> > > > > <http://www.manning.com/bauer3/>
> > > > > JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
> > > > > Spring Batch in Action <http://www.manning.com/templier/>
> > > > > Blog: http://garygregory.wordpress.com
> > > > > Home: http://garygregory.com/
> > > > > Tweet! http://twitter.com/GaryGregory
> > > > >
> > > >
> > >
> >
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r1748015 - in /commons/proper/imaging/trunk/src: changes/changes.xml main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java

James Carman
That's still a tough one (breakfast). Crazy weekend.

It looks like the switch (which to me is more readable) does a hashCode()
call first to find the right place to jump to and then does equals. So,
theoretically it would be faster, especially for large numbers of cases
(unless the compiler is smart enough to recognize when it's effectively a
switch implemented as ifs). It would be interesting to see how well it get
optimized away at runtime though (if I wasn't on vacation tomorrow I might
play with that a bit). It would also be interesting to see the compiled
bytecode side-by-side for the Sun compiler.
On Sun, Jun 12, 2016 at 5:49 PM Gary Gregory <[hidden email]> wrote:

> On Jun 12, 2016 2:45 PM, "James Carman" <[hidden email]>
> wrote:
> >
> > I don't off the top of my head. All I have is anecdotal stuff in my
> brain,
> > but then again I can't remember what I had for lunch yesterday either. :)
>
> How about breakfast this morning? ;-)
>
> Gary
> >
> > On Sun, Jun 12, 2016 at 5:42 PM dbrosIus <[hidden email]>
> wrote:
> >
> > > Sure I got that.  I thought you had a source that explains an if/else
> > > chain getting jitted down to a simple hash jumptable
> > >
> > > -------- Original message --------
> > > From: James Carman <[hidden email]>
> > > Date: 06/12/2016  5:19 PM  (GMT-05:00)
> > > To: Commons Developers List <[hidden email]>
> > > Subject: Re: svn commit: r1748015 - in
> /commons/proper/imaging/trunk/src:
> > > changes/changes.xml
> > > main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > >
> test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> > >
> > > I was referring to JIT.
> > >
> > > On Sun, Jun 12, 2016 at 5:18 PM dbrosIus <[hidden email]>
> > > wrote:
> > >
> > > > Interesting.  Source?
> > > >
> > > > -------- Original message --------
> > > > From: James Carman <[hidden email]>
> > > > Date: 06/12/2016  5:12 PM  (GMT-05:00)
> > > > To: Commons Developers List <[hidden email]>
> > > > Subject: Re: svn commit: r1748015 - in
> /commons/proper/imaging/trunk/src:
> > > > changes/changes.xml
> > > > main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > > >
> test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> > > >
> > > > Doesn't it all compiled down to the same thing? It gets optimized at
> > > > runtime anyway. I would go for the more readable option, unless there
> are
> > > > extremely compelling performance numbers.
> > > >
> > > > On Sun, Jun 12, 2016 at 4:38 PM Benedikt Ritter <[hidden email]>
> > > > wrote:
> > > >
> > > > > Yes, the if-else could also be implemented using a switch
> statement.
> > > But
> > > > is
> > > > > that really more efficient?
> > > > >
> > > > > Benedikt
> > > > >
> > > > > Gary Gregory <[hidden email]> schrieb am So., 12. Juni
> 2016
> um
> > > > > 22:05:
> > > > >
> > > > > > This looks like a candidate for the more efficient switch on
> string.
> > > > > >
> > > > > > Gary
> > > > > >
> > > > > > On Sun, Jun 12, 2016 at 7:47 AM, <[hidden email]> wrote:
> > > > > >
> > > > > > > Author: britter
> > > > > > > Date: Sun Jun 12 14:47:11 2016
> > > > > > > New Revision: 1748015
> > > > > > >
> > > > > > > URL: http://svn.apache.org/viewvc?rev=1748015&view=rev
> > > > > > > Log:
> > > > > > > IMAGING-178: PnmImageParser does not check the validity of
> input
> > > PAM
> > > > > > > header. Thanks to emopers. This also fixes #20 from github.
> > > > > > >
> > > > > > > Modified:
> > > > > > >     commons/proper/imaging/trunk/src/changes/changes.xml
> > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
>
> commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
>
> commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> > > > > > >
> > > > > > > Modified: commons/proper/imaging/trunk/src/changes/changes.xml
> > > > > > > URL:
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
>
> http://svn.apache.org/viewvc/commons/proper/imaging/trunk/src/changes/changes.xml?rev=1748015&r1=1748014&r2=1748015&view=diff
> > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
>
> ==============================================================================
> > > > > > > --- commons/proper/imaging/trunk/src/changes/changes.xml
> (original)
> > > > > > > +++ commons/proper/imaging/trunk/src/changes/changes.xml Sun
> Jun 12
> > > > > > > 14:47:11 2016
> > > > > > > @@ -46,6 +46,9 @@ The <action> type attribute can be add,u
> > > > > > >    <body>
> > > > > > >
> > > > > > >      <release version="1.0" date="TBA" description="First major
> > > > > release">
> > > > > > > +      <action issue="IMAGING-178" dev="britter" type="fix"
> > > > > > > due-to="emopers">
> > > > > > > +        PnmImageParser does not check the validity of input
> PAM
> > > > > header.
> > > > > > > +      </action>
> > > > > > >        <action issue="IMAGING-184" dev="ggregory"
> type="update">
> > > > > > >          Update platform from Java 5 to 7
> > > > > > >        </action>
> > > > > > >
> > > > > > > Modified:
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
>
> commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > > > > > > URL:
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
>
> http://svn.apache.org/viewvc/commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java?rev=1748015&r1=1748014&r2=1748015&view=diff
> > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
>
> ==============================================================================
> > > > > > > ---
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
>
> commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > > > > > > (original)
> > > > > > > +++
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
>
> commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > > > > > > Sun Jun 12 14:47:11 2016
> > > > > > > @@ -157,18 +157,28 @@ public class PnmImageParser extends Imag
> > > > > > >                  final String type = tokenizer.nextToken();
> > > > > > >                  if ("WIDTH".equals(type)) {
> > > > > > >                      seenWidth = true;
> > > > > > > +                    if(!tokenizer.hasMoreTokens())
> > > > > > > +                        throw new ImageReadException("PAM
> header
> > > has
> > > > > no
> > > > > > > WIDTH value");
> > > > > > >                      width =
> > > Integer.parseInt(tokenizer.nextToken());
> > > > > > >                  } else if ("HEIGHT".equals(type)) {
> > > > > > >                      seenHeight = true;
> > > > > > > +                    if(!tokenizer.hasMoreTokens())
> > > > > > > +                        throw new ImageReadException("PAM
> header
> > > has
> > > > > no
> > > > > > > HEIGHT value");
> > > > > > >                      height =
> > > > Integer.parseInt(tokenizer.nextToken());
> > > > > > >                  } else if ("DEPTH".equals(type)) {
> > > > > > >                      seenDepth = true;
> > > > > > > +                    if(!tokenizer.hasMoreTokens())
> > > > > > > +                        throw new ImageReadException("PAM
> header
> > > has
> > > > > no
> > > > > > > DEPTH value");
> > > > > > >                      depth =
> > > Integer.parseInt(tokenizer.nextToken());
> > > > > > >                  } else if ("MAXVAL".equals(type)) {
> > > > > > >                      seenMaxVal = true;
> > > > > > > +                    if(!tokenizer.hasMoreTokens())
> > > > > > > +                        throw new ImageReadException("PAM
> header
> > > has
> > > > > no
> > > > > > > MAXVAL value");
> > > > > > >                      maxVal =
> > > > Integer.parseInt(tokenizer.nextToken());
> > > > > > >                  } else if ("TUPLTYPE".equals(type)) {
> > > > > > >                      seenTupleType = true;
> > > > > > > +                    if(!tokenizer.hasMoreTokens())
> > > > > > > +                        throw new ImageReadException("PAM
> header
> > > has
> > > > > no
> > > > > > > TUPLTYPE value");
> > > > > > >                      tupleType.append(tokenizer.nextToken());
> > > > > > >                  } else if ("ENDHDR".equals(type)) {
> > > > > > >                      break;
> > > > > > >
> > > > > > > Modified:
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
>
> commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> > > > > > > URL:
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
>
> http://svn.apache.org/viewvc/commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java?rev=1748015&r1=1748014&r2=1748015&view=diff
> > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
>
> ==============================================================================
> > > > > > > ---
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
>
> commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> > > > > > > (original)
> > > > > > > +++
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
>
> commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> > > > > > > Sun Jun 12 14:47:11 2016
> > > > > > > @@ -46,4 +46,12 @@ public class PnmImageParserTest {
> > > > > > >      PnmImageParser underTest = new PnmImageParser();
> > > > > > >      underTest.getImageInfo(bytes, params);
> > > > > > >    }
> > > > > > > +
> > > > > > > +  @Test(expected = ImageReadException.class)
> > > > > > > +  public void testGetImageInfo_missingWidthValue() throws
> > > > > > > ImageReadException, IOException {
> > > > > > > +    byte[] bytes = "P7\nWIDTH \n".getBytes(US_ASCII);
> > > > > > > +    Map<String, Object> params = Collections.emptyMap();
> > > > > > > +    PnmImageParser underTest = new PnmImageParser();
> > > > > > > +    underTest.getImageInfo(bytes, params);
> > > > > > > +  }
> > > > > > >  }
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > >
> > > > > >
> > > > > > --
> > > > > > E-Mail: [hidden email] | [hidden email]
> > > > > > Java Persistence with Hibernate, Second Edition
> > > > > > <http://www.manning.com/bauer3/>
> > > > > > JUnit in Action, Second Edition <
> http://www.manning.com/tahchiev/>
> > > > > > Spring Batch in Action <http://www.manning.com/templier/>
> > > > > > Blog: http://garygregory.wordpress.com
> > > > > > Home: http://garygregory.com/
> > > > > > Tweet! http://twitter.com/GaryGregory
> > > > > >
> > > > >
> > > >
> > >
>
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r1748015 - in /commons/proper/imaging/trunk/src: changes/changes.xml main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java

Benedikt Ritter-4
I don't think this discussion is really related to my change introduced in
r1748015. So I leave the optimization topics to our JIT wizards ;-)

Benedikt

James Carman <[hidden email]> schrieb am Mo., 13. Juni 2016 um
00:02 Uhr:

> That's still a tough one (breakfast). Crazy weekend.
>
> It looks like the switch (which to me is more readable) does a hashCode()
> call first to find the right place to jump to and then does equals. So,
> theoretically it would be faster, especially for large numbers of cases
> (unless the compiler is smart enough to recognize when it's effectively a
> switch implemented as ifs). It would be interesting to see how well it get
> optimized away at runtime though (if I wasn't on vacation tomorrow I might
> play with that a bit). It would also be interesting to see the compiled
> bytecode side-by-side for the Sun compiler.
> On Sun, Jun 12, 2016 at 5:49 PM Gary Gregory <[hidden email]>
> wrote:
>
> > On Jun 12, 2016 2:45 PM, "James Carman" <[hidden email]>
> > wrote:
> > >
> > > I don't off the top of my head. All I have is anecdotal stuff in my
> > brain,
> > > but then again I can't remember what I had for lunch yesterday either.
> :)
> >
> > How about breakfast this morning? ;-)
> >
> > Gary
> > >
> > > On Sun, Jun 12, 2016 at 5:42 PM dbrosIus <[hidden email]>
> > wrote:
> > >
> > > > Sure I got that.  I thought you had a source that explains an if/else
> > > > chain getting jitted down to a simple hash jumptable
> > > >
> > > > -------- Original message --------
> > > > From: James Carman <[hidden email]>
> > > > Date: 06/12/2016  5:19 PM  (GMT-05:00)
> > > > To: Commons Developers List <[hidden email]>
> > > > Subject: Re: svn commit: r1748015 - in
> > /commons/proper/imaging/trunk/src:
> > > > changes/changes.xml
> > > > main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > > >
> > test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> > > >
> > > > I was referring to JIT.
> > > >
> > > > On Sun, Jun 12, 2016 at 5:18 PM dbrosIus <[hidden email]>
> > > > wrote:
> > > >
> > > > > Interesting.  Source?
> > > > >
> > > > > -------- Original message --------
> > > > > From: James Carman <[hidden email]>
> > > > > Date: 06/12/2016  5:12 PM  (GMT-05:00)
> > > > > To: Commons Developers List <[hidden email]>
> > > > > Subject: Re: svn commit: r1748015 - in
> > /commons/proper/imaging/trunk/src:
> > > > > changes/changes.xml
> > > > >
> main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > > > >
> > test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> > > > >
> > > > > Doesn't it all compiled down to the same thing? It gets optimized
> at
> > > > > runtime anyway. I would go for the more readable option, unless
> there
> > are
> > > > > extremely compelling performance numbers.
> > > > >
> > > > > On Sun, Jun 12, 2016 at 4:38 PM Benedikt Ritter <
> [hidden email]>
> > > > > wrote:
> > > > >
> > > > > > Yes, the if-else could also be implemented using a switch
> > statement.
> > > > But
> > > > > is
> > > > > > that really more efficient?
> > > > > >
> > > > > > Benedikt
> > > > > >
> > > > > > Gary Gregory <[hidden email]> schrieb am So., 12. Juni
> > 2016
> > um
> > > > > > 22:05:
> > > > > >
> > > > > > > This looks like a candidate for the more efficient switch on
> > string.
> > > > > > >
> > > > > > > Gary
> > > > > > >
> > > > > > > On Sun, Jun 12, 2016 at 7:47 AM, <[hidden email]> wrote:
> > > > > > >
> > > > > > > > Author: britter
> > > > > > > > Date: Sun Jun 12 14:47:11 2016
> > > > > > > > New Revision: 1748015
> > > > > > > >
> > > > > > > > URL: http://svn.apache.org/viewvc?rev=1748015&view=rev
> > > > > > > > Log:
> > > > > > > > IMAGING-178: PnmImageParser does not check the validity of
> > input
> > > > PAM
> > > > > > > > header. Thanks to emopers. This also fixes #20 from github.
> > > > > > > >
> > > > > > > > Modified:
> > > > > > > >     commons/proper/imaging/trunk/src/changes/changes.xml
> > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> >
> >
> commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> >
> >
> commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> > > > > > > >
> > > > > > > > Modified:
> commons/proper/imaging/trunk/src/changes/changes.xml
> > > > > > > > URL:
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> >
> >
> http://svn.apache.org/viewvc/commons/proper/imaging/trunk/src/changes/changes.xml?rev=1748015&r1=1748014&r2=1748015&view=diff
> > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> >
> >
> ==============================================================================
> > > > > > > > --- commons/proper/imaging/trunk/src/changes/changes.xml
> > (original)
> > > > > > > > +++ commons/proper/imaging/trunk/src/changes/changes.xml Sun
> > Jun 12
> > > > > > > > 14:47:11 2016
> > > > > > > > @@ -46,6 +46,9 @@ The <action> type attribute can be add,u
> > > > > > > >    <body>
> > > > > > > >
> > > > > > > >      <release version="1.0" date="TBA" description="First
> major
> > > > > > release">
> > > > > > > > +      <action issue="IMAGING-178" dev="britter" type="fix"
> > > > > > > > due-to="emopers">
> > > > > > > > +        PnmImageParser does not check the validity of input
> > PAM
> > > > > > header.
> > > > > > > > +      </action>
> > > > > > > >        <action issue="IMAGING-184" dev="ggregory"
> > type="update">
> > > > > > > >          Update platform from Java 5 to 7
> > > > > > > >        </action>
> > > > > > > >
> > > > > > > > Modified:
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> >
> >
> commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > > > > > > > URL:
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> >
> >
> http://svn.apache.org/viewvc/commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java?rev=1748015&r1=1748014&r2=1748015&view=diff
> > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> >
> >
> ==============================================================================
> > > > > > > > ---
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> >
> >
> commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > > > > > > > (original)
> > > > > > > > +++
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> >
> >
> commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java
> > > > > > > > Sun Jun 12 14:47:11 2016
> > > > > > > > @@ -157,18 +157,28 @@ public class PnmImageParser extends
> Imag
> > > > > > > >                  final String type = tokenizer.nextToken();
> > > > > > > >                  if ("WIDTH".equals(type)) {
> > > > > > > >                      seenWidth = true;
> > > > > > > > +                    if(!tokenizer.hasMoreTokens())
> > > > > > > > +                        throw new ImageReadException("PAM
> > header
> > > > has
> > > > > > no
> > > > > > > > WIDTH value");
> > > > > > > >                      width =
> > > > Integer.parseInt(tokenizer.nextToken());
> > > > > > > >                  } else if ("HEIGHT".equals(type)) {
> > > > > > > >                      seenHeight = true;
> > > > > > > > +                    if(!tokenizer.hasMoreTokens())
> > > > > > > > +                        throw new ImageReadException("PAM
> > header
> > > > has
> > > > > > no
> > > > > > > > HEIGHT value");
> > > > > > > >                      height =
> > > > > Integer.parseInt(tokenizer.nextToken());
> > > > > > > >                  } else if ("DEPTH".equals(type)) {
> > > > > > > >                      seenDepth = true;
> > > > > > > > +                    if(!tokenizer.hasMoreTokens())
> > > > > > > > +                        throw new ImageReadException("PAM
> > header
> > > > has
> > > > > > no
> > > > > > > > DEPTH value");
> > > > > > > >                      depth =
> > > > Integer.parseInt(tokenizer.nextToken());
> > > > > > > >                  } else if ("MAXVAL".equals(type)) {
> > > > > > > >                      seenMaxVal = true;
> > > > > > > > +                    if(!tokenizer.hasMoreTokens())
> > > > > > > > +                        throw new ImageReadException("PAM
> > header
> > > > has
> > > > > > no
> > > > > > > > MAXVAL value");
> > > > > > > >                      maxVal =
> > > > > Integer.parseInt(tokenizer.nextToken());
> > > > > > > >                  } else if ("TUPLTYPE".equals(type)) {
> > > > > > > >                      seenTupleType = true;
> > > > > > > > +                    if(!tokenizer.hasMoreTokens())
> > > > > > > > +                        throw new ImageReadException("PAM
> > header
> > > > has
> > > > > > no
> > > > > > > > TUPLTYPE value");
> > > > > > > >                      tupleType.append(tokenizer.nextToken());
> > > > > > > >                  } else if ("ENDHDR".equals(type)) {
> > > > > > > >                      break;
> > > > > > > >
> > > > > > > > Modified:
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> >
> >
> commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> > > > > > > > URL:
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> >
> >
> http://svn.apache.org/viewvc/commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java?rev=1748015&r1=1748014&r2=1748015&view=diff
> > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> >
> >
> ==============================================================================
> > > > > > > > ---
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> >
> >
> commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> > > > > > > > (original)
> > > > > > > > +++
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> >
> >
> commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java
> > > > > > > > Sun Jun 12 14:47:11 2016
> > > > > > > > @@ -46,4 +46,12 @@ public class PnmImageParserTest {
> > > > > > > >      PnmImageParser underTest = new PnmImageParser();
> > > > > > > >      underTest.getImageInfo(bytes, params);
> > > > > > > >    }
> > > > > > > > +
> > > > > > > > +  @Test(expected = ImageReadException.class)
> > > > > > > > +  public void testGetImageInfo_missingWidthValue() throws
> > > > > > > > ImageReadException, IOException {
> > > > > > > > +    byte[] bytes = "P7\nWIDTH \n".getBytes(US_ASCII);
> > > > > > > > +    Map<String, Object> params = Collections.emptyMap();
> > > > > > > > +    PnmImageParser underTest = new PnmImageParser();
> > > > > > > > +    underTest.getImageInfo(bytes, params);
> > > > > > > > +  }
> > > > > > > >  }
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > --
> > > > > > > E-Mail: [hidden email] | [hidden email]
> > > > > > > Java Persistence with Hibernate, Second Edition
> > > > > > > <http://www.manning.com/bauer3/>
> > > > > > > JUnit in Action, Second Edition <
> > http://www.manning.com/tahchiev/>
> > > > > > > Spring Batch in Action <http://www.manning.com/templier/>
> > > > > > > Blog: http://garygregory.wordpress.com
> > > > > > > Home: http://garygregory.com/
> > > > > > > Tweet! http://twitter.com/GaryGregory
> > > > > > >
> > > > > >
> > > > >
> > > >
> >
>