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
