4206909 - adding Z_SYNC_FLUSH support to deflaters

[Brandon Long]

In the new Deflater.deflate, you are checking valid flush values against
NO_FLUSH twice (and missing FULL_FLUSH)

The new DeflaterOutputStream.flush, you loop as long as some output is
generated, which can call deflate an extra time.  Normally, code that
does this loops until there is no def.needsInput and the output is less
than the size of the output buffer.  I would have thought it would have
generated extra output, but then the loop would never finish... so I
guess this must work.

Overall, I'm least happy with #4, since I feel it leaves a bug.  flush()
on a stream should flush everything I've written to the stream.  This
bug is that it currently doesn't, and this doesn't fix it.  It makes it
possible for people to fix it (which isn't possible currently without
using a completely separate implementation), but it doesn't fix the bug.

Brandon

On 09/04/09 Xueming Shen uttered the following other thing:
>
> I would like to take the conservative (read compatible) approach for  
> this issue.
>
> (1)expose zlib's flush mode Z_NO_FLUSH, Z_SYNC_FLUSH and Z_FULL_FLUSH
> (2)add deflate(byte[] b, int off, int len, int flush)
> (3)document the existing deflate() continues to use Z_NO_FLUSH
> (4)add DeflaterOutputStream.flush(int flushmode)
>    and leave DeflaterOutputStream.flush() un-touched.
>
> http://cr.openjdk.java.net/~sherman/zipflush/webrev
>
> So the sophisticated Deflater/DeflaterOutputStream user can now use all  
> 3 flush flavors
> by using the newly added APIs explicitly. "naive" Deflater/DOS user  
> (means no need for
> the sync/full_flush) and existing application can continue live with/use  
> the existing API
> without any compatibility concern. The only disadvantage of this  
> approach is the
> "in-direct flush" use scenario (means the DOS is passed around and the  
> inherited flush()
> is invoked by other wrapping classes) will not be benefited from this  
> change, they have
> to override the flush() methold before passing the DOS object around,  
> like what I do
> in the test case Flush()
>
>    static class MyDeflaterOutputStream extends DeflaterOutputStream {
>        public MyDeflaterOutputStream(OutputStream out) {
>            super(out);
> 	}
>
>        public void flush() throws IOException {
>            flush(Deflater.SYNC_FLUSH);
>        }
>    }
>
> A little inconvenient, but seen like worth the  price of being  
> compatible, for now. And we
> still have the choice to do whatever we want to do with this method in  
> the future should the
> feedback show I'm wrong and the compatibility does not matter.
>
> Opinion? If you guys agree (please also review the webrev) I can start  
> the "process".
>
> Sherman
>
>

-- 
 "Quantum Mechanics: The dreams stuff is made of" -- Steven Wright
                                           http://www.fiction.net/blong/