Thanks Andrew. Looks all good to me now.
Cheers, Thomas
On Fri, Mar 29, 2019 at 9:43 AM Andrew Dinn <ad...@redhat.com> wrote:
> On 28/03/2019 17:09, Thomas Stüfe wrote:
> > src/hotspot/share/classfile/javaClasses.cpp
> >
> > Pure nitpicking, but you could remove the member variables too and just
> > set the Unsafe members directly, e.g.:
> >
> > if (fd->name() == vmSymbols::address_size_name()) {
> > mirror->int_field_put(fd->offset(), (jint)sizeof(void*));
> > } else if (fd->name() == vmSymbols::page_size_name()) {
> > mirror->int_field_put(fd->offset(), (jint) os::vm_page_size());
> > ..
> >
> > and so on. But I leave this up to you. This is already better than
> before.
>
> I thought about that but it seemed to me to be better for clarity and
> readability to init all the members in a single block in the constructor
> rather than spread the computations of the field values through the code
> that updates the fields. I think it's easier that way to see where all
> the values are coming from and align them up with the fields in the
> target class.
>
> > + }else {
> > space missing before else
> Yes.
>
> > src/hotspot/share/classfile/vmSymbols.hpp
> > "UNALIGNED_ACCESS" off by one space
>
> Yes.
>
> > src/java.base/share/classes/java/nio/channels/FileChannel.java
> >
> > This may have creeped in from another change.
>
> Ah, yes. That was the next patch that I forgot to back out. I have
> updated the webrev in place to remove it and also fix the whitespace &
> alignment issues.
>
> > Good otherwise.
> Thanks.
>
> regards,
>
>
> Andrew Dinn
> -----------
> Senior Principal Software Engineer
> Red Hat UK Ltd
> Registered in England and Wales under Company Registration No. 03798903
> Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander
>
