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

Coleen Phillimore coleen.phillimore at oracle.com
Fri Dec 11 14:56:35 UTC 2015


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
>>>
>>>
>>>
>>>



More information about the hotspot-runtime-dev mailing list