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

Best
Lance

> 
> 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
lance.ander...@oracle.com

Reply via email to