On 11/03/2012 05:23 PM, Lance Andersen - Oracle wrote:
On Nov 3, 2012, at 11:34 AM, Remi Forax wrote:
On 11/03/2012 03:11 PM, Lance Andersen - Oracle wrote:
I revised the webrev, http://cr.openjdk.java.net/~lancea/8002212/webrev.01,
taking into account the vast majority of Remi's suggestions.
in SerialJavaObject, hasStaticFields doesn't work, the original code doesn't
work because
it only check for fields that are declared static but not for fields that are
by example public static.
private static boolean hasStaticFields(Field[] fields) {
for (Field field : fields) {
if ( Modifier.isStatic(field.getModifiers())) {
return true;
}
}
return false;
}
This may cause compatibility issue because despite the specification, the
original code
will let objects that have a static field to be serialized.
I cannot make the change above as it breaks too many tests and I would prefer
to go with the less is more scenario. As I think I mentioned before, I do not
think the original authors really thought through these classes and thankfully
they are not used much, if at all.
Also, in readObject, if obj is null, the code should throw an IOException
because
it's not possible to create a SerialJavaObject with null has parameter (because
obj.getClass()
that implictly checks null in the constructor).
I made the change to readObject. I did not put an explicit check in the
constructor but will do that under a separate bug
I also added the comment to SerialDataLink and removed the read/writeObject
http://cr.openjdk.java.net/~lancea/8002212/webrev.02
Ok, thumb up.
Best
Lance
cheers,
Rémi
All other classes are Ok for me.
I also added SerialStruct to the webrev.
SerialStruct is Ok for me.
Have a great weekend.
Have a nice weekend too.
Best
Lance
cheers,
Rémi
Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
[email protected]
