RFR(xs): 8145114: const-correctness for ucontext_t* reading functions

[Thomas Stüfe]

Thank you both!

...Thomas

On Fri, Dec 11, 2015 at 3:56 PM, Coleen Phillimore <
coleen.phillimore at oracle.com> wrote:

>
> This looks great to me.   I am sure there'll be more consts, but this will
> improve maintainability of the code.
> Thank you, David, for sponsoring and testing the other platforms.
> Thank you again Thomas for the cleanup.
>
> Coleen
>
>
> On 12/11/15 8:28 AM, Thomas Stüfe wrote:
>
>> Hi David,
>>
>> On Fri, Dec 11, 2015 at 2:11 PM, David Holmes <david.holmes at oracle.com>
>> wrote:
>>
>> On 11/12/2015 10:58 PM, Thomas Stüfe wrote:
>>>
>>> Hi David,
>>>>
>>>> that was an oversight. see new webrev:
>>>>
>>>>
>>>>
>>>> http://cr.openjdk.java.net/~stuefe/webrevs/8145114-const-correctness-ucontext/webrev.01/webrev/
>>>>
>>>> and the delta to webrev 00:
>>>>
>>>>
>>>>
>>>> http://cr.openjdk.java.net/~stuefe/webrevs/8145114-const-correctness-ucontext/webrev.00-01/
>>>>
>>>> That looks fine but ... I'm looking at other uses of ucontext_t and
>>> wondering if they are missing const?
>>>
>>> JavaThread::pd_get_top_frame ?
>>>
>>> ./os/linux/vm/osThread_linux.hpp:  ucontext_t* _ucontext;
>>> ./os/linux/vm/osThread_linux.hpp:  ucontext_t* ucontext() const    {
>>> return _ucontext; }
>>> ./os/linux/vm/osThread_linux.hpp:  void set_ucontext(ucontext_t* ptr)
>>> {
>>> _ucontext = ptr;  }
>>>
>>> etc
>>>
>>>
>>> You are probably right, but I needed to draw the line at some point. I
>> would like to submit the patch as it is (originally I just wanted to fix
>> os::Aix::ucontext_get_pc()).
>>
>> Thanks, Thomas
>>
>>
>>
>>
>> Thanks,
>>> David
>>>
>>> Thanks, Thomas
>>>
>>>> On Fri, Dec 11, 2015 at 11:21 AM, David Holmes <david.holmes at oracle.com
>>>> <mailto:david.holmes at oracle.com>> wrote:
>>>>
>>>>      Hi Thomas,
>>>>
>>>>      In:
>>>>
>>>>      ExtendedPC os::fetch_frame_from_context(const void* ucVoid,
>>>>                           intptr_t** ret_sp, intptr_t** ret_fp) {
>>>>         ExtendedPC  epc;
>>>>         ucontext_t* uc = (ucontext_t*)ucVoid;
>>>>
>>>>
>>>>      is there some reason these ucontext_t's were not made const?
>>>>
>>>>      Thanks,
>>>>      David
>>>>
>>>>      On 11/12/2015 7:57 PM, Thomas Stüfe wrote:
>>>>
>>>>          Hi David,
>>>>
>>>>          On Fri, Dec 11, 2015 at 10:35 AM, David Holmes
>>>>          <david.holmes at oracle.com <mailto:david.holmes at oracle.com>
>>>>          <mailto:david.holmes at oracle.com
>>>>
>>>>          <mailto:david.holmes at oracle.com>>> wrote:
>>>>
>>>>               Hi Thomas,
>>>>
>>>>               This seems okay. To be honest I'm not sure of the
>>>>          usefulness but it
>>>>               certainly isn't harmful :)
>>>>
>>>>               I'll prepare similar changes to our internal platforms and
>>>>          run it
>>>>               all through JPRT.
>>>>
>>>>               Thanks,
>>>>               David
>>>>
>>>>
>>>>               On 11/12/2015 5:16 PM, Thomas Stüfe wrote:
>>>>
>>>>                   Hi all,
>>>>
>>>>                   please review this large-but-trivial change:
>>>>
>>>>                   Bug: https://bugs.openjdk.java.net/browse/JDK-8145114
>>>>                   webrev:
>>>>
>>>>
>>>> http://cr.openjdk.java.net/~stuefe/webrevs/8145114-const-correctness-ucontext/webrev.00/webrev/index.html
>>>>
>>>>                   Even though many files are involved, the changes are
>>>> very
>>>>                   trivial. The
>>>>                   change takes care of declaring ucontext_t pointers as
>>>>          const for
>>>>                   all utility
>>>>                   functions which do not change the context (which are
>>>>          most of them).
>>>>
>>>>                   This fix was motivated by me wasting time in an error
>>>>          situation
>>>>                   on AIX
>>>>                   where I accidentally changed the register values in
>>>> the
>>>>          context
>>>>                   before
>>>>                   printing them to the hs-err file.
>>>>
>>>>                   And, like pulling a thread from a ball of yarn, the
>>>>          change got
>>>>                   bigger and
>>>>                   bigger... like it is once one starts to fix const
>>>>          issues. But
>>>>                   the fixes in
>>>>                   this case involve the bottom functions and should not
>>>>          force any
>>>>                   new casts
>>>>                   upon anyone.
>>>>
>>>>                   I did build the change on the following platforms:
>>>>                   AIX
>>>>                   Linux x64 (with and without ZERO)
>>>>                   Linux ppc
>>>>                   OSX
>>>>                   Solaris Sparc and x64
>>>>
>>>>                   and all did build successfully.
>>>>
>>>>                   Kind Regards, Thomas
>>>>
>>>>
>>>>
>>>>
>>>>
>