The updated fix looks fine to me.
Thanks,
Andrew
On 7/9/2014 10:44 PM, Phil Race wrote:
This now looks fine. Since the fix changed since Andrew approved,
make sure you get a 2nd reviewer.
Also the putback comment must use bug ID 4991647
-phil.
On 7/9/2014 9:53 AM, mikhail cherkasov wrote:
Hi Phil,
I updated testcase:
http://cr.openjdk.java.net/~mcherkas/8041460/webrev.04/
I replaced it with testcase from 499164
Thanks,
Mikhail.
On 7/8/2014 5:30 PM, Phil Race wrote:
This is now bug 4991647 so the test needs to reflect that ..
-phil.
On 7/8/14 5:34 AM, mikhail cherkasov wrote:
Hello Phil,
Please review a new version:
http://cr.openjdk.java.net/~mcherkas/8041460/webrev.03/
I applied changes that you advised, also I removed the comment at
all, because
the new code isn't required clarification.
Thanks,
Mikhail.
On 7/8/2014 8:43 AM, Phil Race wrote:
PS the comments referring to bug id et al seem too much.
I am not sure we need a new comment here just because there was a
bug.
That may be something sustaining may want to do in 'old' releases but
the ongoing clutter for the mainstream release isn't something we
want.
-phil.
On 7/7/14 2:45 PM, Phil Race wrote:
Since its returning an array index into theIHDR_bitDepths array I
think this should be written as
IHDR_bitDepth =IHDR_bitDepths[getEnumeratedAttribute(node,
"bitDepth", IHDR_bitDepths)];
.. but ensuring the line <=80 chars long
Also note that this is a complete duplicate of
https://bugs.openjdk.java.net/browse/JDK-4991647
Always remember to look for duplicates !
So I have closed 8041460 and you should push under 4991647 - once
the code is fixed and reviewed.
I have assigned 4991647 to you.
-phil.
On 6/23/2014 6:09 AM, Andrew Brygin wrote:
Hello Mikhail,
the fix looks fine to me.
Thanks,
Andrew
On 6/9/2014 10:32 PM, mikhail cherkasov wrote:
Hi all,
please review the fix:
http://cr.openjdk.java.net/~mcherkas/8041460/webrev.02/
for the following bug:
https://bugs.openjdk.java.net/browse/JDK-8041460
The problem is that during merging of metadata trees we take
index of IHDR_bitDepths
and set this value to IHDR_bitDepth, but IHDR_bitDepth requires
a real number of bits.
IHDR_bitDepths - store all valid values: "1", "2", "4", "8",
"16", so to get
valid value we need: 1 << index.
Thanks,
Mikhail.
