Hi Olivier,

On Sun, 2006-02-12 at 16:32 +0100, Olivier Jolly wrote:
>   this is my first patch proposition to classpath. I just have my
> paperwork done with FSF and I have created an account on savannah as ojolly.

Thanks. I see you are already active as mauve contributor. 

>   Those patches are both dealing with serialization. One deals with the
> generation of back reference handle in the output stream which was
> forgotten when a proxy was serialized, hence creating an offset in the
> back references for further objects while the other deals with the
> choice of the construtor to use when deserializing an object.

Good catch! A good ChangeLog entry for this part would be:

2006-02-12  Olivier Jolly  <[EMAIL PROTECTED]>

        * java/io/ObjectOutputStream.java (writeClassDescriptor):
        Call assignNewHandle() after writing Proxy class.

>  The first
> constructor of the first concrete non serializable super class was
> selected instead of the first non serializable super class, either
> concrete or abstract. It is reported in bugzilla as bug 14144 (
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=14144 )

Strange. I cannot see why we ever wanted to stop on abstract classes.
The only requirement seems to be that the first non-serial super class
has a public no-arg constructor (which we indeed check). Maybe we didn't
use AllocObject in the past. But we do now.

Note that there is still a comment in the file about this:

    // find the first non-serializable, non-abstract
    // class in clazz's inheritance hierarchy

That should be changed to say what the code does now.
And could you add 2006 to the copyright years in both files?

A good ChangeLog entry for this part would be:

2006-02-12  Olivier Jolly  <[EMAIL PROTECTED]>

        Fixes bug #14144
        * java/io/ObjectInputStream.java (readClassDescriptor):
        Class doesn't have to be abstract for first_nonserial.

If you add Fixes bug #14144 to the CVS commit message (and ChangeLog)
then cvs will automagically notify bugzilla about the commit related to
that bug.

>   Those patches will make pass 2 dedicated tests in mauve in
> java/io/InputOutputStream directory.

Great. And no regressions I assume.

Could you report the patches with the suggestions made above and the
ChangeLog message you want to use. Then I'll add you to the CVS-commit
list and sent an email about it so you can commit these yourself.

Sorry for the micro-review. Just want to explain everything that goes on
in detail.

Thanks,

Mark

Attachment: signature.asc
Description: This is a digitally signed message part

Reply via email to