Quantcast

[9] RFR JDK-8152561: Is it allowed to have zero value for count in TIFFField.createArrayForType() for the rationals

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[9] RFR JDK-8152561: Is it allowed to have zero value for count in TIFFField.createArrayForType() for the rationals

Jayathirth D v

Hello All,

 

Please review the following  fix in JDK9:

 

Bug :              https://bugs.openjdk.java.net/browse/JDK-8152561

Webrev :     http://cr.openjdk.java.net/~jdv/8152561/webrev.00/

 

Issue : There is difference in how we interpret “count” variable passed to TIFFField.createArrayForType(int dataType, int count) and constructor TIFFField(TIFFTag tag, int type, int count, Object data).

 

Root cause : There are certain limitation on what the “count” value should be based on dataType of TIFFTag. We check these conditions in TIFFField(TIFFTag tag, int type, int count, Object data) but we don’t verify all the conditions in TIFFField.createArrayForType(int dataType, int count).

 

Solution : Verify all the required conditions that has to be applied on “count” variable based on datatype of TIFFTag in TIFFField.createArrayForType(int dataType, int count). Also I have made specification changes for TIFFField(TIFFTag tag, int type, int count) which will elaborate on what exceptions will be thrown if we don’t follow same “count” and “dataType” relationship.

 

 

Note : Because of tighter conditions in TIFFField.createArrayForType(int dataType, int count) under the proposed fix 2 JCK tests are failing.

 

1)      api/javax_imageio/plugins/tiff/TIFFField/index.html#TIFFFieldTest : CreateArrayForType_ValidScenario

This is failing because they are passing count values of 0 for TIFFTag.TIFF_RATIONAL & TIFFTag.TIFF_SRATIONAL which will throw IAE. Also it will throw IAE if we pass count value 0 or 2 for TIFFTag.TIFF_IFD_POINTER.

 

2)      api/javax_imageio/plugins/tiff/TIFFField/index.html#ConstructorTests : Constructor02_TagIsNull_ThrowNPE

I think this scenario expects constructor to throw NPE when “tag” is null. But before it checks for “tag” value in TIFFField(TIFFTag tag, int type, int count, Object data) we call TIFFField.createArrayForType(int dataType, int count) to create needed data. So like mentioned in previous JCK test which is failing, we will be throwing IAE in all cases where as test expects us to throw NPE.

 

 

Thanks,

Jay

 

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [9] RFR JDK-8152561: Is it allowed to have zero value for count in TIFFField.createArrayForType() for the rationals

prasanta sadhukhan

Looks good to me.

Only thing is that in the test, if one test fails it will not proceed to the next test. I think we should test all combinations and fail at last with the informations of all failed combinations.

Regards
Prasanta
On 1/11/2017 11:19 PM, Jayathirth D V wrote:

Hello All,

 

Please review the following  fix in JDK9:

 

Bug :              https://bugs.openjdk.java.net/browse/JDK-8152561

Webrev :     http://cr.openjdk.java.net/~jdv/8152561/webrev.00/

 

Issue : There is difference in how we interpret “count” variable passed to TIFFField.createArrayForType(int dataType, int count) and constructor TIFFField(TIFFTag tag, int type, int count, Object data).

 

Root cause : There are certain limitation on what the “count” value should be based on dataType of TIFFTag. We check these conditions in TIFFField(TIFFTag tag, int type, int count, Object data) but we don’t verify all the conditions in TIFFField.createArrayForType(int dataType, int count).

 

Solution : Verify all the required conditions that has to be applied on “count” variable based on datatype of TIFFTag in TIFFField.createArrayForType(int dataType, int count). Also I have made specification changes for TIFFField(TIFFTag tag, int type, int count) which will elaborate on what exceptions will be thrown if we don’t follow same “count” and “dataType” relationship.

 

 

Note : Because of tighter conditions in TIFFField.createArrayForType(int dataType, int count) under the proposed fix 2 JCK tests are failing.

 

1)      api/javax_imageio/plugins/tiff/TIFFField/index.html#TIFFFieldTest : CreateArrayForType_ValidScenario

This is failing because they are passing count values of 0 for TIFFTag.TIFF_RATIONAL & TIFFTag.TIFF_SRATIONAL which will throw IAE. Also it will throw IAE if we pass count value 0 or 2 for TIFFTag.TIFF_IFD_POINTER.

 

2)      api/javax_imageio/plugins/tiff/TIFFField/index.html#ConstructorTests : Constructor02_TagIsNull_ThrowNPE

I think this scenario expects constructor to throw NPE when “tag” is null. But before it checks for “tag” value in TIFFField(TIFFTag tag, int type, int count, Object data) we call TIFFField.createArrayForType(int dataType, int count) to create needed data. So like mentioned in previous JCK test which is failing, we will be throwing IAE in all cases where as test expects us to throw NPE.

 

 

Thanks,

Jay

 


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [9] RFR JDK-8152561: Is it allowed to have zero value for count in TIFFField.createArrayForType() for the rationals

Jayathirth D v

Hi Prasanta,

 

Thanks for your review.

Now I am forcing all testcases to run even if any initial testcase fails.

I have divided all 4 test cases into different functions and I am throwing proper RuntimeException based on what test case is failing.

 

Please find updated webrev for review :

http://cr.openjdk.java.net/~jdv/8152561/webrev.01/

 

Thanks,

Jay

 

From: Prasanta Sadhukhan
Sent: Tuesday, January 17, 2017 12:48 PM
To: Jayathirth D V; Philip Race; Brian Burkhalter; 2d-dev
Subject: Re: [OpenJDK 2D-Dev] [9] RFR JDK-8152561: Is it allowed to have zero value for count in TIFFField.createArrayForType() for the rationals

 

Looks good to me.

Only thing is that in the test, if one test fails it will not proceed to the next test. I think we should test all combinations and fail at last with the informations of all failed combinations.

Regards
Prasanta

On 1/11/2017 11:19 PM, Jayathirth D V wrote:

Hello All,

 

Please review the following  fix in JDK9:

 

Bug :              https://bugs.openjdk.java.net/browse/JDK-8152561

Webrev :     http://cr.openjdk.java.net/~jdv/8152561/webrev.00/

 

Issue : There is difference in how we interpret “count” variable passed to TIFFField.createArrayForType(int dataType, int count) and constructor TIFFField(TIFFTag tag, int type, int count, Object data).

 

Root cause : There are certain limitation on what the “count” value should be based on dataType of TIFFTag. We check these conditions in TIFFField(TIFFTag tag, int type, int count, Object data) but we don’t verify all the conditions in TIFFField.createArrayForType(int dataType, int count).

 

Solution : Verify all the required conditions that has to be applied on “count” variable based on datatype of TIFFTag in TIFFField.createArrayForType(int dataType, int count). Also I have made specification changes for TIFFField(TIFFTag tag, int type, int count) which will elaborate on what exceptions will be thrown if we don’t follow same “count” and “dataType” relationship.

 

 

Note : Because of tighter conditions in TIFFField.createArrayForType(int dataType, int count) under the proposed fix 2 JCK tests are failing.

 

1)      api/javax_imageio/plugins/tiff/TIFFField/index.html#TIFFFieldTest : CreateArrayForType_ValidScenario

This is failing because they are passing count values of 0 for TIFFTag.TIFF_RATIONAL & TIFFTag.TIFF_SRATIONAL which will throw IAE. Also it will throw IAE if we pass count value 0 or 2 for TIFFTag.TIFF_IFD_POINTER.

 

2)      api/javax_imageio/plugins/tiff/TIFFField/index.html#ConstructorTests : Constructor02_TagIsNull_ThrowNPE

I think this scenario expects constructor to throw NPE when “tag” is null. But before it checks for “tag” value in TIFFField(TIFFTag tag, int type, int count, Object data) we call TIFFField.createArrayForType(int dataType, int count) to create needed data. So like mentioned in previous JCK test which is failing, we will be throwing IAE in all cases where as test expects us to throw NPE.

 

 

Thanks,

Jay

 

 

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [9] RFR JDK-8152561: Is it allowed to have zero value for count in TIFFField.createArrayForType() for the rationals

prasanta sadhukhan

Looks good to me.

Regards
Prasanta
On 1/18/2017 2:33 PM, Jayathirth D V wrote:

Hi Prasanta,

 

Thanks for your review.

Now I am forcing all testcases to run even if any initial testcase fails.

I have divided all 4 test cases into different functions and I am throwing proper RuntimeException based on what test case is failing.

 

Please find updated webrev for review :

http://cr.openjdk.java.net/~jdv/8152561/webrev.01/

 

Thanks,

Jay

 

From: Prasanta Sadhukhan
Sent: Tuesday, January 17, 2017 12:48 PM
To: Jayathirth D V; Philip Race; Brian Burkhalter; 2d-dev
Subject: Re: [OpenJDK 2D-Dev] [9] RFR JDK-8152561: Is it allowed to have zero value for count in TIFFField.createArrayForType() for the rationals

 

Looks good to me.

Only thing is that in the test, if one test fails it will not proceed to the next test. I think we should test all combinations and fail at last with the informations of all failed combinations.

Regards
Prasanta

On 1/11/2017 11:19 PM, Jayathirth D V wrote:

Hello All,

 

Please review the following  fix in JDK9:

 

Bug :              https://bugs.openjdk.java.net/browse/JDK-8152561

Webrev :     http://cr.openjdk.java.net/~jdv/8152561/webrev.00/

 

Issue : There is difference in how we interpret “count” variable passed to TIFFField.createArrayForType(int dataType, int count) and constructor TIFFField(TIFFTag tag, int type, int count, Object data).

 

Root cause : There are certain limitation on what the “count” value should be based on dataType of TIFFTag. We check these conditions in TIFFField(TIFFTag tag, int type, int count, Object data) but we don’t verify all the conditions in TIFFField.createArrayForType(int dataType, int count).

 

Solution : Verify all the required conditions that has to be applied on “count” variable based on datatype of TIFFTag in TIFFField.createArrayForType(int dataType, int count). Also I have made specification changes for TIFFField(TIFFTag tag, int type, int count) which will elaborate on what exceptions will be thrown if we don’t follow same “count” and “dataType” relationship.

 

 

Note : Because of tighter conditions in TIFFField.createArrayForType(int dataType, int count) under the proposed fix 2 JCK tests are failing.

 

1)      api/javax_imageio/plugins/tiff/TIFFField/index.html#TIFFFieldTest : CreateArrayForType_ValidScenario

This is failing because they are passing count values of 0 for TIFFTag.TIFF_RATIONAL & TIFFTag.TIFF_SRATIONAL which will throw IAE. Also it will throw IAE if we pass count value 0 or 2 for TIFFTag.TIFF_IFD_POINTER.

 

2)      api/javax_imageio/plugins/tiff/TIFFField/index.html#ConstructorTests : Constructor02_TagIsNull_ThrowNPE

I think this scenario expects constructor to throw NPE when “tag” is null. But before it checks for “tag” value in TIFFField(TIFFTag tag, int type, int count, Object data) we call TIFFField.createArrayForType(int dataType, int count) to create needed data. So like mentioned in previous JCK test which is failing, we will be throwing IAE in all cases where as test expects us to throw NPE.

 

 

Thanks,

Jay

 

 


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [9] RFR JDK-8152561: Is it allowed to have zero value for count in TIFFField.createArrayForType() for the rationals

Brian Burkhalter-2
In reply to this post by Jayathirth D v
Hi Jay,

I think line 910 of TIFFField needs to state “and count != 1”. Also the copyright year should now be 2017.

OK to make these changes without another webrev: +1.

Thanks,

Brian

On Jan 18, 2017, at 1:03 AM, Jayathirth D V <[hidden email]> wrote:

Please find updated webrev for review :

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [9] RFR JDK-8152561: Is it allowed to have zero value for count in TIFFField.createArrayForType() for the rationals

Philip Race
} else if((dataType == TIFFTag.TIFF_RATIONAL

I know that someone (ahem Brian ;-)) has a lot of "if(" code in here
but please make any new ones "if ("

Other than that, +1

-phil.


On 1/24/17, 1:08 PM, Brian Burkhalter wrote:
Hi Jay,

I think line 910 of TIFFField needs to state “and count != 1”. Also the copyright year should now be 2017.

OK to make these changes without another webrev: +1.

Thanks,

Brian

On Jan 18, 2017, at 1:03 AM, Jayathirth D V <[hidden email]> wrote:

Please find updated webrev for review :

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [9] RFR JDK-8152561: Is it allowed to have zero value for count in TIFFField.createArrayForType() for the rationals

Jayathirth D v

Hi Phil & Brian,

 

Thanks for your review.

FYI, I have updated the webrev to include your suggestions :

http://cr.openjdk.java.net/~jdv/8152561/webrev.02/

 

Regards,

Jay

 

From: Philip Race
Sent: Wednesday, January 25, 2017 11:01 AM
To: Brian Burkhalter
Cc: Jayathirth D V; Prasanta Sadhukhan; 2d-dev
Subject: Re: [OpenJDK 2D-Dev] [9] RFR JDK-8152561: Is it allowed to have zero value for count in TIFFField.createArrayForType() for the rationals

 

} else if((dataType == TIFFTag.TIFF_RATIONAL
 
I know that someone (ahem Brian ;-)) has a lot of "if(" code in here
but please make any new ones "if ("
 
Other than that, +1
 
-phil.



On 1/24/17, 1:08 PM, Brian Burkhalter wrote:

Hi Jay,

 

I think line 910 of TIFFField needs to state “and count != 1”. Also the copyright year should now be 2017.

 

OK to make these changes without another webrev: +1.

 

Thanks,

 

Brian

 

On Jan 18, 2017, at 1:03 AM, Jayathirth D V <[hidden email]> wrote:



Please find updated webrev for review :

 

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [9] RFR JDK-8152561: Is it allowed to have zero value for count in TIFFField.createArrayForType() for the rationals

Brian Burkhalter-2
Hi Jay,

+1

Thanks,

Brian

On Jan 24, 2017, at 11:26 PM, Jayathirth D V <[hidden email]> wrote:

Thanks for your review.
FYI, I have updated the webrev to include your suggestions :
 

Loading...