Review request 8002212 - adding read/writeObject to additional SerialXXX classes

[Lance Andersen - Oracle]

On Nov 3, 2012, at 11:14 AM, Remi Forax wrote:

> On 11/03/2012 01:46 AM, Lance Andersen - Oracle wrote:
>> Hi Remi,
>> 
> [...]
> 
>>> In SerialDataLink, do you really need readObject/writeObject given
>>> that you call the default implementations.
>> I thought about that but had decided to add them for consistency
> 
> The serialization implementation needs to create metadata associated with readObject/writeObject,
> so if we can avoid to implement them, we reduce the runtime memory footprint a little.
> I would prefer to have a comment saying that default serialization is Ok here.

OK,  I will just comment the methods out and add a comment as you suggest
> 
>>> in SerialJavaObject, in equals, you should declare a local variable like in SerialDataLink.equals,
>>> even if the local varialble is used once, it's more readable.
>> OK
>>> Also like in SerialArray.equals, you can do a return directly instead of if(...) return true.
>>> in clone(), you can use the diamond syntax,
>>> sjo.chain = new Vector<>(chain);
>> Yeah, long story, but I forgot to reset to diamond syntax (will tell you over a beer sometime :-) )
> 
> sure, now or you have to visit Paris or I have to go to NY :)

Or Boston, where I am based or perhaps a JavaOne ;-)
> 
>>> in setWarning(), you can use the diamond syntax as the original source does.
>>> and in readObject, you can use the diamond syntax too.
>> OK
>>> In readObject, you forget to throw an exception if there are some static fields
>>> in getClass().getFields() as the constructor does
>>> (this code can be moved in a private static method).
>> I thought about that, but figured since it was already through that check when the object was created, I did not think it required repeating in the readObject method
> 
> A serialization stream can be forged to encode a SerialJavaObject that references an object that have a static field without creating a SerialJavaObject in memory.

True and there are tests in the test suite that basically do that.  Anyways, I added that check in the revised webrev that I pushed earlier.
> 
>>> Also, you should add a comment that because you call getClass() on obj,
>>> there is an implicit null check.
>> Would it be better to put an null check in explicitly?
> 
> As you prefer :)
> 
> [...]
> 
>> Thank you again, will send an update webrev over the weekend
>> 
>> Best
>> Lance
> 
> cheers,
> Rémi

Best
Lance
-------------- next part --------------

Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
Lance.Andersen at oracle.com