On Tue, 7 Feb 2023 07:07:54 GMT, Alan Bateman <al...@openjdk.org> wrote:

>>> Hi @AlanBateman ,
>>> with latest changes (doing inflate/deinflate) test passes over s390 and x86 
>>> as well. Please take a look now.
>> 
>> Good. One thing to try is to just deflate/inflate into out1/out2, no need 
>> for the intermediate BAOS, try this:
>> 
>> 
>> --- a/test/jdk/java/util/zip/DeInflate.java
>> +++ b/test/jdk/java/util/zip/DeInflate.java
>> @@ -1,5 +1,5 @@
>>  /*
>> - * Copyright (c) 2011, 2018, Oracle and/or its affiliates. All rights 
>> reserved.
>> + * Copyright (c) 2011, 2023, Oracle and/or its affiliates. All rights 
>> reserved.
>>   * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>>   *
>>   * This code is free software; you can redistribute it and/or modify it
>> @@ -127,11 +127,19 @@ public class DeInflate {
>>  
>>          def.setInput(in, 0, len);
>>          def.finish();
>> -        int m = def.deflate(out1);
>> +        int m = 0;
>> +        while (!def.finished()) {
>> +            int remaining = out1.length - m;
>> +            m += def.deflate(out1, m, remaining);
>> +        }
>>  
>>          Inflater inf = new Inflater(nowrap);
>>          inf.setInput(out1, 0, m);
>> -        int n = inf.inflate(out2);
>> +        int n = 0;
>> +        while (!inf.finished()) {
>> +            int remaining = out2.length - n;
>> +            n += inf.inflate(out2, n, remaining);
>> +        }
>>  
>>          if (n != len ||
>>              !Arrays.equals(Arrays.copyOf(in, len), Arrays.copyOf(out2, 
>> len)) ||
>
>> @AlanBateman should we proceed with the current changes then ?
> 
> The BAOS in your current proposal shouldn't be needed so I suspect there is 
> something else "interesting" with this zlib implementation.  For the patch 
> above, can you remove the checking for remaining == 0 and print out the value 
> from deflate at each iterate of the loop. I'm wondering if it returns 0 
> before def.finished() returns true.

> Hi @AlanBateman, what should be our next step for this PR :)

I don't think we have a good handle on the issue. It's clear that this zlib 
implementation is a bit different but I'm concerned that your proposed change 
to the test is masking an issue. I say this because the `check` method is 
changed to not use the out1 and out2 as the test intended. I'm also concerned 
that the code fragment you pasted may be the combination of two patches, I 
can't otherwise explain the ArrayIndexOutOfBoundsException you reported. Would 
it be possible to re-run with the follow change as I'm like to see again what 
the failure is:


--- a/test/jdk/java/util/zip/DeInflate.java
+++ b/test/jdk/java/util/zip/DeInflate.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2011, 2023, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -127,11 +127,19 @@ public class DeInflate {
 
         def.setInput(in, 0, len);
         def.finish();
-        int m = def.deflate(out1);
+        int m = 0;
+        while (!def.finished()) {
+            int remaining = out1.length - m;
+            m += def.deflate(out1, m, remaining);
+        }
 
         Inflater inf = new Inflater(nowrap);
         inf.setInput(out1, 0, m);
-        int n = inf.inflate(out2);
+        int n = 0;
+        while (!inf.finished()) {
+            int remaining = out2.length - n;
+            n += inf.inflate(out2, n, remaining);
+        }
 
         if (n != len ||
             !Arrays.equals(Arrays.copyOf(in, len), Arrays.copyOf(out2, len)) ||

-------------

PR: https://git.openjdk.org/jdk/pull/12283

Reply via email to