[jdk9] RFR: 8163517: Various cleanup in java.io code
Claes Redestad
claes.redestad at oracle.com
Thu Aug 18 18:27:28 UTC 2016
On 2016-08-18 20:23, Ivan Gerasimov wrote:
> Hi Claes!
>
> Thank you for looking into this.
>
>
> On 18.08.2016 15:25, Claes Redestad wrote:
>> On 2016-08-16 19:20, Ivan Gerasimov wrote:
>>>> ObjectStreamClass:
>>>>
>>>> I wonder if the entire getPackageName is replaceable with
>>>> Class.getPackageName()? If so, ditch the method completely and use
>>>> Class.getPackageName everywhere. This may complicate backporting to
>>>> JDK
>>>> 8 though.
>>>
>>> Unfortunately, Class.getPackageName() cannot handle arrays, so I'll
>>> leave it as is.
>>
>> There are a number of utility methods around that could be reused,
>> e.g., jdk.internal.reflect.Reflection.isSameClassPackage handles both
>> arrays and
>> primitive types and could replace both packageEquals and
>> getPackageName in
>> ObjectStreamClass (which already use j.i.r.Reflection for other
>> things). Making
>> isSameClassPackage in Reflection public should be non-controversial,
>> I hope.
>>
>
> Yes, thanks for the pointer!
> That's right, there are several similar methods in different packages.
> I see that the third option is
> sun.invoke.util.VerifyAccess.isSamePackage / getPackageName, but it
> also has its nuances.
>
> I guess that keeping ObjectStreamClass.getPackageName() separated
> would be the simplest approach: it minimizes the chances of regression
> and doesn't make it harder to do future modifications to
> Reflection.isSameClassPackage, if it is ever needed.
>
> If there are no objections, I'd like to push the latest variant of the
> fix, which is at
> http://cr.openjdk.java.net/~igerasim/8163517/01/webrev/
Go ahead, I'm just musing about the opportunity to consolidate further,
which is better kept as a follow-up anyhow.
Rather than breaking into Reflection we should likely add an internally
public API with a more well-defined contract, ensuring that it addresses
arrays, primitives etc, and then Reflection, VerifyAccess and
ObjectStream could consolidate their use to such a method.
Thanks!
/Claes
More information about the core-libs-dev
mailing list