RFR (L): 8208671: Runtime, JFR, Serviceability changes to allow enabling -Wreorder

Harold David Seigel harold.seigel at oracle.com
Fri Aug 3 12:03:37 UTC 2018


Hi Thomas,

Thanks for the new webrev.

The original vm_operations.hpp had a call to _setter() for 
VM_FindDeadlocks(bool concurrent_locks) that got  removed.  (The first 
VM_FindDeadlocks() had a call to _setter().  The second 
VM_FindDeadlocks() did not.)

    - VM_FindDeadlocks(bool concurrent_locks) :
    _concurrent_locks(concurrent_locks), _out(NULL), _deadlocks(NULL),
    _setter() {};
    - VM_FindDeadlocks(outputStream* st) : _concurrent_locks(true),
    _out(st), _deadlocks(NULL) {};
    + VM_FindDeadlocks(bool concurrent_locks) :
    _concurrent_locks(concurrent_locks), _deadlocks(NULL), _out(NULL) {};
    + VM_FindDeadlocks(outputStream* st) : _concurrent_locks(true),
    _deadlocks(NULL), _out(st) {};


Also, in elfFile.cpp there is an added call to _elfHdr():

    - _string_tables(NULL), _symbol_tables(NULL), _funcDesc_table(NULL),
    - _next(NULL), _status(NullDecoder::no_error),
    - _shdr_string_table(NULL), _file(NULL), _filepath(NULL) {
    + _next(NULL), _filepath(NULL), _file(NULL), _elfHdr(),
    + _symbol_tables(NULL), _string_tables(NULL),
    _shdr_string_table(NULL), _funcDesc_table(NULL),
    + _status(NullDecoder::no_error) {

I don't need to see a new webrev.

Thanks, Harold
On 8/3/2018 4:55 AM, Thomas Schatzl wrote:
> Hi Harold,
>
>    thanks for your review.
>
> On Thu, 2018-08-02 at 13:18 -0400, Harold David Seigel wrote:
>> Hi Thomas,
>>
>> The change looks good!  Just a few comments.
>>
>> In vm_operations.hpp, there an added call to _setter() in
>> VM_FindDeadlocks(outputStream* st).  Is that call needed to
>> initialize
>> _setter() properly?
>>
>>      - VM_FindDeadlocks(bool concurrent_locks) :
>>      _concurrent_locks(concurrent_locks), _out(NULL),
>> _deadlocks(NULL),
>>      _setter() {};
>>      - VM_FindDeadlocks(outputStream* st) : _concurrent_locks(true),
>>      _out(st), _deadlocks(NULL) {};
>>      + VM_FindDeadlocks(bool concurrent_locks) :
>>      _concurrent_locks(concurrent_locks), _deadlocks(NULL),
>> _out(NULL),
>>      _setter() {};
>>      + VM_FindDeadlocks(outputStream* st) : _concurrent_locks(true),
>>      _deadlocks(NULL), _out(st), _setter() {};
>>
>> In elfFuncDescTable.cpp, there an added call to _status(), is that
>> call
>> needed to initialize _status properly?
>>
>>      - _file(file), _index(index), _section(file, shdr) {
>>      + _section(file, shdr), _file(file), _index(index), _status() {
>    fixed (including David's findings) in
> http://cr.openjdk.java.net/~tschatzl/8208671/webrev.1 (full)
> http://cr.openjdk.java.net/~tschatzl/8208671/webrev.0_to_1 (diff)
>
> Thanks,
>    Thomas
>



More information about the hotspot-dev mailing list