Code review request, JDK-8010814, More buffers are stored or returned without cloning

[Weijun Wang]

On 5/16/13 5:52 PM, Xuelei Fan wrote:
> On 5/16/2013 5:22 PM, Weijun Wang wrote:
>> Hi Xuelei
>>
>> I'm busy on something else, so probably have no time (or cannot
>> concentrate) to read in details.
>>
> Not urgent fix, so please review these request only when you available.
>
>> In my opinion, there are several cases as to whether to clone or not:
>>
>> 1. MUST NOT clone, because the value must be shared so that change at
>> one side appears on the other side.
>>
>> 2. MUST clone, public available methods that leads to security issues.
>>
>> 3. So so, although public methods, cannot be used to do bad things.
>>
>> 4. Not an issue. Internal methods.
>>
>> In the patch, are the first two cases #1 or #3.
>>
> It's #2.

I mean the cases in BerDecoder and BerEncoder. You only add comments

    // shared buffer, be careful to use this class

>
> For #1 and #3, I added a few comment lines in 8010815.
>
>> Sorry I also have no time to read
>>
>>    JDK-8010814, More buffers are stored or returned without cloning
>>    http://cr.openjdk.java.net/~xuelei/8010815/webrev.00/
>>
>> but I am not sure if those env cases belong to #1.
>>
> I think both fixes are for #2.

I'm not sure.

Take Continuation for example, the class is used to continue a previous 
method. For me it looks like the environment should be shared between them.

Can you give an example that shows the environment should not be shared?

Thanks
Max


>
> Xuelei
>
>> Thanks
>> Max
>>
>>
>> On 5/16/13 5:08 PM, Xuelei Fan wrote:
>>> Hi,
>>>
>>> There is another fix to avoid the use of mutable objects.
>>>
>>> webrev: http://cr.openjdk.java.net/~xuelei/8010814/webrev.00/
>>>
>>> Thanks,
>>> Xuelei
>>>
>