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

[Harold David Seigel]

Looks good!

Thanks, Harold


On 8/3/2018 9:30 AM, Thomas Schatzl wrote:
> Hi Harold,
>
> On Fri, 2018-08-03 at 08:03 -0400, Harold David Seigel wrote:
>> 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) {};
> So the original code had the call to the (default) initializer and the
> other did not... I changed it to as before.
>
>> 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.
> Removed. I updated the .0_to_1 and .1 webrevs.
>
> Thanks,
>    Thomas
>
>> 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
>>>
>>