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

[Thomas Stüfe]

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