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

[David Holmes]

On 11/12/2015 11:28 PM, Thomas Stüfe wrote:
> Hi David,
>
> On Fri, Dec 11, 2015 at 2:11 PM, David Holmes <david.holmes at oracle.com
> <mailto: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()).

Okay I'll push what is there.

Thanks,
David

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