RFR JDK-8153353: HPACK implementation

Chris Hegarty chris.hegarty at oracle.com
Mon Apr 18 14:55:07 UTC 2016


On 18/04/16 15:30, Pavel Rappo wrote:
>
>> On 13 Apr 2016, at 10:10, Chris Hegarty <chris.hegarty at oracle.com> wrote:
>>
>> A general comment; Quite a number of classes, especially Encoder and Decoder,
>> would benefit from a few relatively lightweight comments, e.g. IntegerReader
>
> I would like to address it after the initial push.

That's fine.

 > If possible. Other than that,
> I have updated the implementation:
>
>      http://cr.openjdk.java.net/~prappo/8153353/webrev.01/

I'm happy with this version.

-Chris.

> There's also a javadoc now:
>
>      http://cr.openjdk.java.net/~prappo/8153353/javadoc.01/
>
> Removed some FIXMEs and TODOs, updated tests.
>
>> StringReader.java
>>
>>   reset() can probably just reset its readers, regardless of huffman, since the
>>   readers are always guaranteed to be present ( and reset is minimal ).
>>
>>   General comment; in many cases it would be useful to capture, at least, the ‘state’
>>   when throwing InternalError.
>
> Done.
>
>> IntegerReader.java
>>
>>   reset() assumes that there is no bugs in the code. Should is assign 0 to r,
>>   value, and maxValue as well.
>
> It will be done in the next invocation of 'configure'.
>
>> Q: is it necessary to call configure after reset?
>
> Yes.
>
>> ISO_8859_1.java
>>
>>   Sorry, I don’ get 'outstanding'. Are you catching RuntimeException somewhere,
>>   or do you expect a user of the Decoder to do some special exception handling ?
>
> Removed. It was a leftover from a failure-atomic behaviour, which is not needed here.
>
>> HpackException.java
>>
>>   Do you need this ??? ( FIXME comment )
>
> Removed, thanks. Instead, a sandwich is thrown:
>
>      UncheckedIOException(ProtocolException).
>


More information about the net-dev mailing list