RFR (L) 8213501 : Deploy ExceptionJniWrapper for a few tests
Jean Christophe Beyler
jcbeyler at google.com
Tue Apr 2 16:59:11 UTC 2019
Hi all,
Friendly ping on this one, I know I've used up quite a bit of time on it;
my apologies again :)
Jc
On Tue, Mar 26, 2019 at 12:37 PM Jean Christophe Beyler <jcbeyler at google.com>
wrote:
> Hi all,
>
> My apologies again for this one. This has been a bit tricky to get in and
> has fell off my priority list due to other issues/commitments but I'm back
> to finishing this up and the next webrevs regarding this work :)
>
> Problem is that we change years so the copyrights changed on some of these
> and there were a few problems with various architectures/build systems that
> made the testing fail on the submit repo.
>
> So I offer two webrevs:
>
> - The incremental from the last LGTM stamped one:
> http://cr.openjdk.java.net/~jcbeyler/8213501/webrev.06_07/
>
> - The full webrev that cleaned up a few problems for windows and solaris
> and now passes the submit repo:
> http://cr.openjdk.java.net/~jcbeyler/8213501/webrev.07/
>
> And again, my sincere apologies that it took me SO long to get back to
> this but I had to work through the random submit repo failures and some of
> them took time for me to debug (thanks Serguei for your help :-)),
> Jc
>
> On Tue, Jan 22, 2019 at 6:03 PM Alex Menkov <alexey.menkov at oracle.com>
> wrote:
>
>> +1
>>
>> --alex
>>
>> On 01/22/2019 10:29, JC Beyler wrote:
>> > Thanks Paul!
>> >
>> > Anybody else for the review for version 6?
>> >
>> > Webrev: http://cr.openjdk.java.net/~jcbeyler/8213501/webrev.06/
>> > <http://cr.openjdk.java.net/~jcbeyler/8213501/webrev.06/>
>> > Bug: https://bugs.openjdk.java.net/browse/JDK-8213501
>> > <https://bugs.openjdk.java.net/browse/JDK-8213501>
>> >
>> > Thanks,
>> > Jc
>> >
>> > On Tue, Jan 22, 2019 at 6:10 AM Hohensee, Paul <hohensee at amazon.com
>> > <mailto:hohensee at amazon.com>> wrote:
>> >
>> > Lgtm :)
>> >
>> > Paul
>> >
>> > On 1/14/19, 7:46 AM, "hotspot-dev on behalf of JC Beyler"
>> > <hotspot-dev-bounces at openjdk.java.net
>> > <mailto:hotspot-dev-bounces at openjdk.java.net> on behalf of
>> > jcbeyler at google.com <mailto:jcbeyler at google.com>> wrote:
>> >
>> > Hi all,
>> >
>> > Friendly ping on this one, I know that it has been a long
>> > process with back
>> > and forths, to which I apologize...
>> >
>> > But is there any way I could get a final LGTM for version 6?
>> >
>> > Webrev:
>> http://cr.openjdk.java.net/~jcbeyler/8213501/webrev.06/
>> > Bug: https://bugs.openjdk.java.net/browse/JDK-8213501
>> >
>> > Thanks!
>> > Jc
>> >
>> > On Tue, Jan 8, 2019 at 10:05 AM JC Beyler <jcbeyler at google.com
>> > <mailto:jcbeyler at google.com>> wrote:
>> >
>> > > Happy new year all!
>> > >
>> > > Could I get a final LGTM for version 6?
>> > >
>> > > Webrev:
>> http://cr.openjdk.java.net/~jcbeyler/8213501/webrev.06/
>> > > Bug: https://bugs.openjdk.java.net/browse/JDK-8213501
>> > >
>> > > Thanks!
>> > > Jc
>> > >
>> > > On Mon, Dec 17, 2018 at 8:43 AM JC Beyler
>> > <jcbeyler at google.com <mailto:jcbeyler at google.com>> wrote:
>> > >
>> > >> Hi all,
>> > >>
>> > >> I don't believe I got actual LGTM for this version:
>> > >>
>> > >>
>> > >> Webrev:
>> http://cr.openjdk.java.net/~jcbeyler/8213501/webrev.06/
>> > >> Bug: https://bugs.openjdk.java.net/browse/JDK-8213501
>> > >>
>> > >>
>> > >> It removed the namespaces and uses explicit static instead
>> :)
>> > >>
>> > >> Thanks!
>> > >> Jc
>> > >>
>> > >> On Wed, Dec 12, 2018 at 8:06 PM JC Beyler
>> > <jcbeyler at google.com <mailto:jcbeyler at google.com>> wrote:
>> > >>
>> > >>> So did I Alexey but with David & Serguei preferring static,
>> > it seems
>> > >>> more reasonable to go down their route :-)
>> > >>>
>> > >>> So here is the latest webrev with static instead of an
>> > anonymous
>> > >>> namespace:
>> > >>>
>> > >>> Webrev:
>> http://cr.openjdk.java.net/~jcbeyler/8213501/webrev.06/
>> > >>> Bug: https://bugs.openjdk.java.net/browse/JDK-8213501
>> > >>>
>> > >>> Let me know what you think, can I get a webrev 06 review?
>> > >>>
>> > >>> Thanks!
>> > >>> Jc
>> > >>>
>> > >>> On Wed, Dec 12, 2018 at 3:10 PM Alex Menkov
>> > <alexey.menkov at oracle.com <mailto:alexey.menkov at oracle.com>>
>> > >>> wrote:
>> > >>>
>> > >>>> Hm..
>> > >>>> I considered unnamed namespaces "C++ style" (and static
>> > globals as "C
>> > >>>> style").
>> > >>>> Static globals were deprecated in C++ (but some time ago
>> the
>> > >>>> deprecation
>> > >>>> was reverted).
>> > >>>>
>> > >>>> --alex
>> > >>>>
>> > >>>> On 12/12/2018 13:55, serguei.spitsyn at oracle.com
>> > <mailto:serguei.spitsyn at oracle.com> wrote:
>> > >>>> > Agreed.
>> > >>>> >
>> > >>>> > Thanks,
>> > >>>> > Serguei
>> > >>>> >
>> > >>>> >
>> > >>>> > On 12/12/18 13:52, David Holmes wrote:
>> > >>>> >> FWIW I think namespaces are overkill in all of this
>> > test code and
>> > >>>> just
>> > >>>> >> obfuscates things - the declaration is easily missed. A
>> > static
>> > >>>> >> variable in a .cpp is clearly a global variable to the
>> > file.
>> > >>>> >>
>> > >>>> >> Cheers,
>> > >>>> >> David
>> > >>>> >>
>> > >>>> >>
>> > >>>> >>
>> > >>>> >> On 13/12/2018 5:37 am, serguei.spitsyn at oracle.com
>> > <mailto:serguei.spitsyn at oracle.com> wrote:
>> > >>>> >>> Hi Jc,
>> > >>>> >>>
>> > >>>> >>>
>> > >>>> >>> On 12/11/18 21:16, JC Beyler wrote:
>> > >>>> >>>> Hi all,
>> > >>>> >>>>
>> > >>>> >>>> Here is the new webrev with the TEST.groups change.
>> > Serguei, let
>> > >>>> me
>> > >>>> >>>> know if I convinced you with the static vs anonymous
>> > namespaces or
>> > >>>> >>>> if you'd still rather have a "static" for now :-)
>> > >>>> >>>
>> > >>>> >>>
>> > >>>> >>> What do you think about this post? :
>> > >>>> >>>
>> > >>>>
>> >
>> https://stackoverflow.com/questions/11623451/static-vs-non-static-variables-in-namespace
>> > >>>> >>>
>> > >>>> >>>
>> > >>>> >>>>
>> > >>>> >>>> Webrev:
>> > http://cr.openjdk.java.net/~jcbeyler/8213501/webrev.05/
>> > >>>> >>>>
>> > <http://cr.openjdk.java.net/%7Ejcbeyler/8213501/webrev.05/>
>> > >>>> >>>> Bug:
>> https://bugs.openjdk.java.net/browse/JDK-8213501
>> > >>>> >>>
>> > >>>> >>> The update looks fine.
>> > >>>> >>>
>> > >>>> >>> Thanks,
>> > >>>> >>> Serguei
>> > >>>> >>>
>> > >>>> >>>
>> > >>>> >>> Thanks,
>> > >>>> >>> Serguei
>> > >>>> >>>
>> > >>>> >>>>
>> > >>>> >>>> Thanks again for the reviews!
>> > >>>> >>>> Jc
>> > >>>> >>>>
>> > >>>> >>>> On Mon, Dec 10, 2018 at 3:10 PM JC Beyler
>> > <jcbeyler at google.com <mailto:jcbeyler at google.com>
>> > >>>> >>>> <mailto:jcbeyler at google.com
>> > <mailto:jcbeyler at google.com>>> wrote:
>> > >>>> >>>>
>> > >>>> >>>> Hi Serguei,
>> > >>>> >>>>
>> > >>>> >>>> Yes basically it is equivalent :) I can put them
>> > in but they
>> > >>>> are
>> > >>>> >>>> not required. The norm actually wanted to
>> > deprecate it but then
>> > >>>> >>>> remembered that C compatibility would require the
>> > static
>> > >>>> key-word
>> > >>>> >>>> for this case [1]
>> > >>>> >>>>
>> > >>>> >>>> So, really, they are not required here and will
>> > amount to the
>> > >>>> same
>> > >>>> >>>> thing: only that file can refer to them and you
>> > cannot get to
>> > >>>> them
>> > >>>> >>>> without a globally available method to return a
>> > pointer to them
>> > >>>> >>>> (ie same as a static variable in C).
>> > >>>> >>>>
>> > >>>> >>>> I can put static if it makes it easier to see
>> > but, by being in
>> > >>>> an
>> > >>>> >>>> anonymous namespace they are only available for
>> > the file's
>> > >>>> >>>> translation unit. For example:
>> > >>>> >>>>
>> > >>>> >>>> $ cat main.cpp
>> > >>>> >>>>
>> > >>>> >>>> int totally_global;
>> > >>>> >>>> static int explictly_static;
>> > >>>> >>>>
>> > >>>> >>>> namespace {
>> > >>>> >>>> int implicitly_static;
>> > >>>> >>>> }
>> > >>>> >>>>
>> > >>>> >>>> void foo();
>> > >>>> >>>> int main() {
>> > >>>> >>>> foo();
>> > >>>> >>>> }
>> > >>>> >>>>
>> > >>>> >>>> $ g++ -O3 main.cpp -c
>> > >>>> >>>> $ nm main.o
>> > >>>> >>>> U _GLOBAL_OFFSET_TABLE_
>> > >>>> >>>> 0000000000000000 T main
>> > >>>> >>>> 0000000000000000 B totally_global
>> > >>>> >>>> U _Z3foov
>> > >>>> >>>>
>> > >>>> >>>> As you can see, the static and anonymous
>> > namespace variables
>> > >>>> are
>> > >>>> >>>> not in the file due to not being used. If you
>> > were to use them,
>> > >>>> >>>> you'd see them show up as something like:
>> > >>>> >>>> 0000000000000008 b _ZL17explicitly_static
>> > >>>> >>>> 0000000000000004 b
>> > _ZN12_GLOBAL__N_117implicitly_staticE
>> > >>>> >>>>
>> > >>>> >>>> Where again, it shows that it is mangling the
>> > names so that no
>> > >>>> >>>> external usage can happen without tinkering.
>> > >>>> >>>>
>> > >>>> >>>> Hopefully that helps :-),
>> > >>>> >>>> Jc
>> > >>>> >>>>
>> > >>>> >>>> [1]
>> > >>>> >>>>
>> > http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1012
>> > >>>> >>>>
>> > >>>> >>>>
>> > >>>> >>>> On Mon, Dec 10, 2018 at 2:04 PM
>> > serguei.spitsyn at oracle.com <mailto:serguei.spitsyn at oracle.com>
>> > >>>> >>>> <mailto:serguei.spitsyn at oracle.com
>> > <mailto:serguei.spitsyn at oracle.com>> <
>> > >>>> serguei.spitsyn at oracle.com <mailto:
>> serguei.spitsyn at oracle.com>
>> > >>>> >>>> <mailto:serguei.spitsyn at oracle.com
>> > <mailto:serguei.spitsyn at oracle.com>>> wrote:
>> > >>>> >>>>
>> > >>>> >>>> Hi Jc,
>> > >>>> >>>>
>> > >>>> >>>> I had little experience with the C++
>> namespaces.
>> > >>>> >>>> My understanding is that static in this
>> > context should mean
>> > >>>> >>>> internal linkage.
>> > >>>> >>>>
>> > >>>> >>>> Thanks,
>> > >>>> >>>> Serguei
>> > >>>> >>>>
>> > >>>> >>>>
>> > >>>> >>>> On 12/10/18 13:57, JC Beyler wrote:
>> > >>>> >>>>> Hi Serguei,
>> > >>>> >>>>>
>> > >>>> >>>>> The variables and functions are in a
>> > anonymous namespace;
>> > >>>> my
>> > >>>> >>>>> understanding of C++ is that this is
>> > equivalent to
>> > >>>> putting it
>> > >>>> >>>>> as static.Hence, I didn't add them there.
>> > Does that make
>> > >>>> >>>>> sense?
>> > >>>> >>>>>
>> > >>>> >>>>> Thanks!
>> > >>>> >>>>> Jc
>> > >>>> >>>>>
>> > >>>> >>>>> On Mon, Dec 10, 2018 at 1:33 PM
>> > >>>> serguei.spitsyn at oracle.com <mailto:
>> serguei.spitsyn at oracle.com>
>> > >>>> >>>>> <mailto:serguei.spitsyn at oracle.com
>> > <mailto:serguei.spitsyn at oracle.com>>
>> > >>>> >>>>> <serguei.spitsyn at oracle.com
>> > <mailto:serguei.spitsyn at oracle.com>
>> > >>>> >>>>> <mailto:serguei.spitsyn at oracle.com
>> > <mailto:serguei.spitsyn at oracle.com>>> wrote:
>> > >>>> >>>>>
>> > >>>> >>>>> Hi Jc,
>> > >>>> >>>>>
>> > >>>> >>>>> It looks good in general.
>> > >>>> >>>>> One question though.
>> > >>>> >>>>>
>> > >>>> >>>>>
>> > >>>>
>> >
>> http://cr.openjdk.java.net/%7Ejcbeyler/8213501/webrev.03a_04/test/hotspot/jtreg/vmTestbase/nsk/share/ExceptionCheckingJniEnv/exceptionjni001/exceptionjni001.cpp.html
>> > >>>> >>>>>
>> > >>>> >>>>>
>> > >>>> >>>>> I wonder if the variables and functions
>> > have to be
>> > >>>> static.
>> > >>>> >>>>>
>> > >>>> >>>>> Thanks,
>> > >>>> >>>>> Serguei
>> > >>>> >>>>>
>> > >>>> >>>>>
>> > >>>> >>>>> On 12/5/18 11:36, JC Beyler wrote:
>> > >>>> >>>>>> Hi all,
>> > >>>> >>>>>>
>> > >>>> >>>>>> My apologies to having to come back for
>> > another
>> > >>>> review
>> > >>>> >>>>>> for this change: I ran into a snag when
>> > trying to
>> > >>>> pull
>> > >>>> >>>>>> the latest changes compared to the base
>> > I was working
>> > >>>> >>>>>> on. I basically forgot that there was
>> > an issue with
>> > >>>> >>>>>> snprintf and that I had solved it via
>> > JDK-8213622.
>> > >>>> >>>>>>
>> > >>>> >>>>>> Could I have a new review of this
>> webrev:
>> > >>>> >>>>>> Webrev:
>> > >>>> >>>>>>
>> http://cr.openjdk.java.net/~jcbeyler/8213501/webrev.04/
>> > >>>> >>>>>>
>> > <http://cr.openjdk.java.net/%7Ejcbeyler/8213501/webrev.04/>
>> > >>>> >>>>>> Bug:
>> > >>>> https://bugs.openjdk.java.net/browse/JDK-8213501
>> > >>>> >>>>>> Incremental from the port of webrev.03
>> > that got
>> > >>>> LGTMs:
>> > >>>> >>>>>>
>> > http://cr.openjdk.java.net/~jcbeyler/8213501/webrev.03a_04/
>> > >>>> >>>>>>
>> > <http://cr.openjdk.java.net/%7Ejcbeyler/8213501/webrev.03a_04/>
>> > >>>> >>>>>>
>> > >>>> >>>>>> A few comments on this because it took
>> > me a while to
>> > >>>> get
>> > >>>> >>>>>> things in a state I thought was good:
>> > >>>> >>>>>> - I had to implement an itoa method,
>> > do we have
>> > >>>> >>>>>> something like that in the test base
>> > (remember that
>> > >>>> >>>>>> JDK-8213622 could not use sprintf due
>> > to being in the
>> > >>>> >>>>>> test code)?
>> > >>>> >>>>>>
>> > >>>> >>>>>> - The differences here compared to
>> > the one you all
>> > >>>> >>>>>> reviewed are:
>> > >>>> >>>>>> - I found that adding to the
>> > strlen/memcpy
>> > >>>> error
>> > >>>> >>>>>> prone and thought that I would try to
>> > make it less
>> > >>>> so.
>> > >>>> >>>>>> If you want to compare, I extended the
>> > strlen/memcpy
>> > >>>> >>>>>> with the new format to show you if you
>> > prefer [1]
>> > >>>> >>>>>> - Note that the diff
>> > between the "old
>> > >>>> >>>>>> extended way from [1]" to the webrev.04
>> > can be found
>> > >>>> >>>>>> in [2]
>> > >>>> >>>>>>
>> > >>>> >>>>>> - I added a test to test the
>> > exception wrapper
>> > >>>> in
>> > >>>> >>>>>> tests :); I'm not sure it is deemed
>> > useful or not but
>> > >>>> >>>>>> helped me assure myself that I was not
>> > doing things
>> > >>>> >>>>>> wrong; you can find the base test file
>> > here [3];
>> > >>>> should
>> > >>>> >>>>>> we have this or not? (I know that
>> > normally we don't
>> > >>>> add
>> > >>>> >>>>>> tests to vmTestbase but thought this
>> > might be an
>> > >>>> >>>>>> exception)
>> > >>>> >>>>>>
>> > >>>> >>>>>> Thanks for your help and my apologies
>> > for the snag,
>> > >>>> >>>>>> Jc
>> > >>>> >>>>>>
>> > >>>> >>>>>> [1]:
>> > >>>> >>>>>>
>> > >>>>
>> >
>> http://cr.openjdk.java.net/~jcbeyler/8213501/webrev.03a/test/hotspot/jtreg/vmTestbase/nsk/share/jni/ExceptionCheckingJniEnv.cpp.udiff.html
>> > >>>> >>>>>>
>> > >>>> >>>>>> <
>> > >>>>
>> >
>> http://cr.openjdk.java.net/%7Ejcbeyler/8213501/webrev.03a/test/hotspot/jtreg/vmTestbase/nsk/share/jni/ExceptionCheckingJniEnv.cpp.udiff.html
>> >
>> > >>>>
>> > >>>> >>>>>>
>> > >>>> >>>>>> [2]:
>> > >>>> >>>>>>
>> > http://cr.openjdk.java.net/~jcbeyler/8213501/webrev.03a_04
>> > >>>> >>>>>>
>> > <http://cr.openjdk.java.net/%7Ejcbeyler/8213501/webrev.03a_04>
>> > >>>> >>>>>> [3]
>> > >>>> >>>>>>
>> > >>>>
>> >
>> http://cr.openjdk.java.net/~jcbeyler/8213501/webrev.04/test/hotspot/jtreg/vmTestbase/nsk/share/ExceptionCheckingJniEnv/exceptionjni001/exceptionjni001.cpp.html
>> > >>>> >>>>>>
>> > >>>> >>>>>> <
>> > >>>>
>> >
>> http://cr.openjdk.java.net/%7Ejcbeyler/8213501/webrev.04/test/hotspot/jtreg/vmTestbase/nsk/share/ExceptionCheckingJniEnv/exceptionjni001/exceptionjni001.cpp.html
>> >
>> > >>>>
>> > >>>> >>>>>>
>> > >>>> >>>>>>
>> > >>>> >>>>>> On Mon, Dec 3, 2018 at 11:29 PM 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:
>> > >>>> >>>>>>
>> > >>>> >>>>>> Looks fine to me.
>> > >>>> >>>>>>
>> > >>>> >>>>>> Thanks,
>> > >>>> >>>>>> David
>> > >>>> >>>>>>
>> > >>>> >>>>>> On 4/12/2018 4:04 pm, JC Beyler
>> wrote:
>> > >>>> >>>>>> > Hi both,
>> > >>>> >>>>>> >
>> > >>>> >>>>>> > Thanks for the reviews! Since
>> > Serguei did not
>> > >>>> >>>>>> insist on get_basename, I
>> > >>>> >>>>>> > went for get_dirname since the
>> > method is a
>> > >>>> local
>> > >>>> >>>>>> static method and won't
>> > >>>> >>>>>> > have its name start spreading, I
>> > think it's ok
>> > >>>> too.
>> > >>>> >>>>>> >
>> > >>>> >>>>>> > For the naming of the local
>> > variable, the idea
>> > >>>> >>>>>> initially was to use the
>> > >>>> >>>>>> > same name as the local variable
>> > for JNIEnv
>> > >>>> already
>> > >>>> >>>>>> used to reduce the
>> > >>>> >>>>>> > code change. Since I'm now adding
>> > the line
>> > >>>> macro
>> > >>>> >>>>>> at the end anyway, this
>> > >>>> >>>>>> > does not matter anymore so I
>> > converged all
>> > >>>> local
>> > >>>> >>>>>> variables to "jni".
>> > >>>> >>>>>> >
>> > >>>> >>>>>> > So, without further ado, here is
>> > the new
>> > >>>> version:
>> > >>>> >>>>>> > Webrev:
>> > >>>> >>>>>>
>> http://cr.openjdk.java.net/~jcbeyler/8213501/webrev.03/
>> > >>>> >>>>>>
>> > <http://cr.openjdk.java.net/%7Ejcbeyler/8213501/webrev.03/>
>> > >>>> >>>>>> > Bug:
>> > >>>> >>>>>> https://bugs.openjdk.java.net/browse/JDK-8213501
>> > >>>> >>>>>> >
>> > >>>> >>>>>> > This passes the various tests
>> > changed by the
>> > >>>> >>>>>> webrev on my dev machine.
>> > >>>> >>>>>> >
>> > >>>> >>>>>> > Let me know what you think,
>> > >>>> >>>>>> > Jc
>> > >>>> >>>>>> >
>> > >>>> >>>>>> > On Mon, Dec 3, 2018 at 8:40 PM
>> > >>>> >>>>>> serguei.spitsyn at oracle.com
>> > <mailto:serguei.spitsyn at oracle.com>
>> > >>>> >>>>>> <mailto:serguei.spitsyn at oracle.com
>> > <mailto:serguei.spitsyn at oracle.com>>
>> > >>>> >>>>>> >
>> > <mailto:serguei.spitsyn at oracle.com <mailto:
>> serguei.spitsyn at oracle.com>
>> > >>>> >>>>>> <mailto:serguei.spitsyn at oracle.com
>> > <mailto:serguei.spitsyn at oracle.com>>>
>> > >>>> >>>>>> <serguei.spitsyn at oracle.com
>> > <mailto:serguei.spitsyn at oracle.com>
>> > >>>> >>>>>> <mailto:serguei.spitsyn at oracle.com
>> > <mailto:serguei.spitsyn at oracle.com>>
>> > >>>> >>>>>> >
>> > <mailto:serguei.spitsyn at oracle.com <mailto:
>> serguei.spitsyn at oracle.com>
>> > >>>> >>>>>> <mailto:serguei.spitsyn at oracle.com
>> > <mailto:serguei.spitsyn at oracle.com>>>> wrote:
>> > >>>> >>>>>> >
>> > >>>> >>>>>> > On 12/3/18 20:15, Chris
>> > Plummer wrote:
>> > >>>> >>>>>> > > Hi JC,
>> > >>>> >>>>>> > >
>> > >>>> >>>>>> > > Overall it looks good. A
>> > few naming nits
>> > >>>> >>>>>> thought:
>> > >>>> >>>>>> > >
>> > >>>> >>>>>> > > In bi01t001.cpp, why have
>> > you declared
>> > >>>> the
>> > >>>> >>>>>> > ExceptionCheckingJniEnvPtr
>> > >>>> >>>>>> > > using jni_env(jni).
>> > Elsewhere you use
>> > >>>> >>>>>> jni(jni_env) and rename the
>> > >>>> >>>>>> > > method argument passed in
>> > from jni to
>> > >>>> >>>>>> jni_env.
>> > >>>> >>>>>> > >
>> > >>>> >>>>>> > > Related to this, I also
>> > noticed in some
>> > >>>> >>>>>> files that already are using
>> > >>>> >>>>>> > >
>> > ExceptionCheckingJniEnvPtr, such as
>> > >>>> >>>>>> CharArrayCriticalLocker.cpp, you
>> > >>>> >>>>>> > > delcared it as
>> > env(jni_env). So that
>> > >>>> means
>> > >>>> >>>>>> there are 3 different
>> > >>>> >>>>>> > names
>> > >>>> >>>>>> > > you have used for the
>> > >>>> >>>>>> ExceptionCheckingJniEnvPtr local
>> > variable.
>> > >>>> >>>>>> > They
>> > >>>> >>>>>> > > should be consistent.
>> > >>>> >>>>>> > >
>> > >>>> >>>>>> > > Also, can you rename
>> > get_basename() to
>> > >>>> >>>>>> get_dirname()? I know Serguei
>> > >>>> >>>>>> > > suggested get_basename() a
>> > while back,
>> > >>>> but
>> > >>>> >>>>>> unless "basename" is
>> > >>>> >>>>>> > > commonly used for this
>> > purpose, I think
>> > >>>> >>>>>> "dirname" is more self
>> > >>>> >>>>>> > > explanatory.
>> > >>>> >>>>>> >
>> > >>>> >>>>>> > In general, I'm Okay with
>> > get_dirname().
>> > >>>> >>>>>> > Just to mention dirname can
>> > be both short
>> > >>>> or
>> > >>>> >>>>>> full, so it is a little
>> > >>>> >>>>>> > confusing as well.
>> > >>>> >>>>>> > It is the reason why the
>> > get_basename() was
>> > >>>> >>>>>> suggested.
>> > >>>> >>>>>> > However, I do not insist on
>> > get_basename()
>> > >>>> nor
>> > >>>> >>>>>> get_full_dirname(). :)
>> > >>>> >>>>>> >
>> > >>>> >>>>>> > Thanks,
>> > >>>> >>>>>> > Serguei
>> > >>>> >>>>>> >
>> > >>>> >>>>>> >
>> > >>>> >>>>>> > > thanks,
>> > >>>> >>>>>> > >
>> > >>>> >>>>>> > > Chris
>> > >>>> >>>>>> > >
>> > >>>> >>>>>> > > On 12/2/18 10:29 PM, David
>> > Holmes wrote:
>> > >>>> >>>>>> > >> Hi Jc,
>> > >>>> >>>>>> > >>
>> > >>>> >>>>>> > >> I've been lurking on this
>> > one and have
>> > >>>> had
>> > >>>> >>>>>> a look through. I'm okay
>> > >>>> >>>>>> > >> with the FatalError
>> > approach for the
>> > >>>> tests
>> > >>>> >>>>>> - we don't expect
>> > >>>> >>>>>> > anything
>> > >>>> >>>>>> > >> to go wrong in a well
>> > written test in a
>> > >>>> >>>>>> correctly functioning VM.
>> > >>>> >>>>>> > >>
>> > >>>> >>>>>> > >> Thanks,
>> > >>>> >>>>>> > >> David
>> > >>>> >>>>>> > >>
>> > >>>> >>>>>> > >>
>> > >>>> >>>>>> > >>
>> > >>>> >>>>>> > >> On 3/12/2018 3:24 pm, JC
>> > Beyler wrote:
>> > >>>> >>>>>> > >>> Hi all,
>> > >>>> >>>>>> > >>>
>> > >>>> >>>>>> > >>> Would someone on the GC
>> > or runtime
>> > >>>> team
>> > >>>> >>>>>> be motivated to give
>> > >>>> >>>>>> > this a
>> > >>>> >>>>>> > >>> review? :)
>> > >>>> >>>>>> > >>>
>> > >>>> >>>>>> > >>> It would be much
>> > appreciated!
>> > >>>> >>>>>> > >>>
>> > >>>> >>>>>> > >>> Webrev:
>> > >>>> >>>>>>
>> http://cr.openjdk.java.net/~jcbeyler/8213501/webrev.02/
>> > >>>> >>>>>>
>> > <http://cr.openjdk.java.net/%7Ejcbeyler/8213501/webrev.02/>
>> > >>>> >>>>>> > >>> Bug:
>> > >>>> >>>>>> https://bugs.openjdk.java.net/browse/JDK-8213501
>> > >>>> >>>>>> > >>>
>> > >>>> >>>>>> > >>> Thanks for your help,
>> > >>>> >>>>>> > >>> Jc
>> > >>>> >>>>>> > >>>
>> > >>>> >>>>>> > >>> On Tue, Nov 27, 2018 at
>> > 4:36 PM JC
>> > >>>> Beyler
>> > >>>> >>>>>> <jcbeyler at google.com
>> > <mailto:jcbeyler at google.com> <mailto:jcbeyler at google.com
>> > <mailto:jcbeyler at google.com>
>> > >>>> >
>> > >>>> >>>>>> > <mailto:jcbeyler at google.com
>> > <mailto:jcbeyler at google.com>
>> > >>>> >>>>>> <mailto:jcbeyler at google.com
>> > <mailto:jcbeyler at google.com>>>
>> > >>>> >>>>>> > >>>
>> > <mailto:jcbeyler at google.com <mailto:jcbeyler at google.com>
>> > >>>> >>>>>> <mailto:jcbeyler at google.com
>> > <mailto:jcbeyler at google.com>>
>> > >>>> >>>>>> <mailto:jcbeyler at google.com
>> > <mailto:jcbeyler at google.com>
>> > >>>> >>>>>> <mailto:jcbeyler at google.com
>> > <mailto:jcbeyler at google.com>>>>> wrote:
>> > >>>> >>>>>> > >>>
>> > >>>> >>>>>> > >>> Hi Chris,
>> > >>>> >>>>>> > >>>
>> > >>>> >>>>>> > >>> Yes I was waiting
>> > for another
>> > >>>> review
>> > >>>> >>>>>> since you had explicitly
>> > >>>> >>>>>> > >>> asked :)
>> > >>>> >>>>>> > >>>
>> > >>>> >>>>>> > >>> And sounds good that
>> > when someone
>> > >>>> >>>>>> from GC or runtime gives a
>> > >>>> >>>>>> > >>> review,
>> > >>>> >>>>>> > >>> I'll wait for your
>> > full review on
>> > >>>> the
>> > >>>> >>>>>> webrev.02!
>> > >>>> >>>>>> > >>>
>> > >>>> >>>>>> > >>> Thanks again for
>> > your help,
>> > >>>> >>>>>> > >>> Jc
>> > >>>> >>>>>> > >>>
>> > >>>> >>>>>> > >>>
>> > >>>> >>>>>> > >>> On Tue, Nov 27, 2018
>> > at 12:48 PM
>> > >>>> >>>>>> Chris Plummer
>> > >>>> >>>>>> > >>>
>> > <chris.plummer at oracle.com <mailto:chris.plummer at oracle.com>
>> > >>>> >>>>>> <mailto:chris.plummer at oracle.com
>> > <mailto:chris.plummer at oracle.com>>
>> > >>>> >>>>>> <mailto:chris.plummer at oracle.com
>> > <mailto:chris.plummer at oracle.com>
>> > >>>> >>>>>> <mailto:chris.plummer at oracle.com
>> > <mailto:chris.plummer at oracle.com>>>
>> > >>>> >>>>>> > <mailto:
>> chris.plummer at oracle.com
>> > <mailto:chris.plummer at oracle.com>
>> > >>>> >>>>>> <mailto:chris.plummer at oracle.com
>> > <mailto:chris.plummer at oracle.com>>
>> > >>>> >>>>>> <mailto:chris.plummer at oracle.com
>> > <mailto:chris.plummer at oracle.com>
>> > >>>> >>>>>> <mailto:chris.plummer at oracle.com
>> > <mailto:chris.plummer at oracle.com>>>>>
>> > >>>> >>>>>> > wrote:
>> > >>>> >>>>>> > >>>
>> > >>>> >>>>>> > >>> Hi JC,
>> > >>>> >>>>>> > >>>
>> > >>>> >>>>>> > >>> I think it would
>> > be good to
>> > >>>> get a
>> > >>>> >>>>>> review from the gc or
>> > >>>> >>>>>> > runtime
>> > >>>> >>>>>> > >>> teams, since
>> > this also affects
>> > >>>> >>>>>> their tests.
>> > >>>> >>>>>> > >>>
>> > >>>> >>>>>> > >>> Also, once we
>> > are settled on
>> > >>>> this
>> > >>>> >>>>>> FatalError approach,
>> > >>>> >>>>>> > I still
>> > >>>> >>>>>> > >>> need to give
>> > your webrev-02 a
>> > >>>> >>>>>> full review. I only
>> > >>>> >>>>>> > skimmed over
>> > >>>> >>>>>> > >>> parts of it (I
>> > did look at all
>> > >>>> >>>>>> the changes in webrevo-01).
>> > >>>> >>>>>> > >>>
>> > >>>> >>>>>> > >>> thanks,
>> > >>>> >>>>>> > >>>
>> > >>>> >>>>>> > >>> Chris
>> > >>>> >>>>>> > >>>
>> > >>>> >>>>>> > >>> On 11/27/18
>> 8:58 AM,
>> > >>>> >>>>>> serguei.spitsyn at oracle.com
>> > <mailto:serguei.spitsyn at oracle.com>
>> > >>>> >>>>>> <mailto:serguei.spitsyn at oracle.com
>> > <mailto:serguei.spitsyn at oracle.com>>
>> > >>>> >>>>>> >
>> > <mailto:serguei.spitsyn at oracle.com <mailto:
>> serguei.spitsyn at oracle.com>
>> > >>>> >>>>>> <mailto:serguei.spitsyn at oracle.com
>> > <mailto:serguei.spitsyn at oracle.com>>>
>> > >>>> >>>>>> > >>>
>> > <mailto:serguei.spitsyn at oracle.com <mailto:
>> serguei.spitsyn at oracle.com>
>> > >>>> >>>>>> <mailto:serguei.spitsyn at oracle.com
>> > <mailto:serguei.spitsyn at oracle.com>>
>> > >>>> >>>>>> >
>> > <mailto:serguei.spitsyn at oracle.com <mailto:
>> serguei.spitsyn at oracle.com>
>> > >>>> >>>>>> <mailto:serguei.spitsyn at oracle.com
>> > <mailto:serguei.spitsyn at oracle.com>>>> wrote:
>> > >>>> >>>>>> > >>>> Hi Jc,
>> > >>>> >>>>>> > >>>>
>> > >>>> >>>>>> > >>>> I've already
>> > reviewed this
>> > >>>> too.
>> > >>>> >>>>>> > >>>>
>> > >>>> >>>>>> > >>>> Thanks,
>> > >>>> >>>>>> > >>>> Serguei
>> > >>>> >>>>>> > >>>>
>> > >>>> >>>>>> > >>>>
>> > >>>> >>>>>> > >>>> On 11/27/18
>> > 06:56, JC Beyler
>> > >>>> >>>>>> wrote:
>> > >>>> >>>>>> > >>>>> Thanks Chris,
>> > >>>> >>>>>> > >>>>>
>> > >>>> >>>>>> > >>>>> Anybody else motivated
>> > to look at
>> > >>>> this
>> > >>>> >>>>>> and review it? :)
>> > >>>> >>>>>> > >>>>> Jc
>> > >>>> >>>>>> > >>>>>
>> > >>>> >>>>>> > >>>>> On Mon, Nov
>> > 26, 2018 at
>> > >>>> 1:26 PM
>> > >>>> >>>>>> Chris Plummer
>> > >>>> >>>>>> > >>>>>
>> > <chris.plummer at oracle.com <mailto:chris.plummer at oracle.com>
>> > >>>> >>>>>> <mailto:chris.plummer at oracle.com
>> > <mailto:chris.plummer at oracle.com>>
>> > >>>> >>>>>> > <mailto:
>> chris.plummer at oracle.com
>> > <mailto:chris.plummer at oracle.com>
>> > >>>> >>>>>> <mailto:chris.plummer at oracle.com
>> > <mailto:chris.plummer at oracle.com>>>
>> > >>>> >>>>>> <mailto:chris.plummer at oracle.com
>> > <mailto:chris.plummer at oracle.com>
>> > >>>> >>>>>> <mailto:chris.plummer at oracle.com
>> > <mailto:chris.plummer at oracle.com>>
>> > >>>> >>>>>> > <mailto:
>> chris.plummer at oracle.com
>> > <mailto:chris.plummer at oracle.com>
>> > >>>> >>>>>> <mailto:chris.plummer at oracle.com
>> > <mailto:chris.plummer at oracle.com>>>>>
>> > >>>> >>>>>> > >>>>> wrote:
>> > >>>> >>>>>> > >>>>>
>> > >>>> >>>>>> > >>>>> Hi JC,
>> > >>>> >>>>>> > >>>>>
>> > >>>> >>>>>> > >>>>> I'm ok with the
>> > FatalError approach,
>> > >>>> >>>>>> but would
>> > >>>> >>>>>> > like to
>> > >>>> >>>>>> > >>>>> hear opinions from
>> > others also.
>> > >>>> >>>>>> > >>>>>
>> > >>>> >>>>>> > >>>>> thanks,
>> > >>>> >>>>>> > >>>>>
>> > >>>> >>>>>> > >>>>> Chris
>> > >>>> >>>>>> > >>>>>
>> > >>>> >>>>>> > >>>>> On 11/21/18 8:19 AM,
>> > JC Beyler
>> > >>>> wrote:
>> > >>>> >>>>>> > >>>>>> Hi Chris,
>> > >>>> >>>>>> > >>>>>>
>> > >>>> >>>>>> > >>>>>> Thanks
>> > for taking the
>> > >>>> time
>> > >>>> >>>>>> to look at it and yes you
>> > >>>> >>>>>> > >>>>>> have
>> > raised exactly why
>> > >>>> >>>>>> the webrev is between two
>> > >>>> >>>>>> > >>>>>> worlds:
>> > in cases where
>> > >>>> a
>> > >>>> >>>>>> fatal error on failure is
>> > >>>> >>>>>> > >>>>>> wanted,
>> > should we
>> > >>>> simplify
>> > >>>> >>>>>> the code to remove
>> > >>>> >>>>>> > the return
>> > >>>> >>>>>> > >>>>>> tests
>> > since we do them
>> > >>>> >>>>>> internally? Now that I've
>> > >>>> >>>>>> > looked
>> > >>>> >>>>>> > >>>>>> around
>> > for non-fatal
>> > >>>> >>>>>> cases, I think the answer
>> > >>>> >>>>>> > is yes,
>> > >>>> >>>>>> > >>>>>> it
>> > simplifies the code
>> > >>>> >>>>>> while maintaining the checks.
>> > >>>> >>>>>> > >>>>>>
>> > >>>> >>>>>> > >>>>>> I looked
>> > a bit and it
>> > >>>> >>>>>> seems that I can't find
>> > >>>> >>>>>> > easily a
>> > >>>> >>>>>> > >>>>>> case
>> > where the test
>> > >>>> >>>>>> accepts a JNI failure to
>> > >>>> >>>>>> > then move
>> > >>>> >>>>>> > >>>>>> on.
>> > Therefore, perhaps,
>> > >>>> >>>>>> for now, the fail with a
>> > >>>> >>>>>> > Fatal
>> > >>>> >>>>>> > >>>>>> is enough
>> > and we can
>> > >>>> work
>> > >>>> >>>>>> on the tests to clean
>> > >>>> >>>>>> > them up?
>> > >>>> >>>>>> > >>>>>>
>> > >>>> >>>>>> > >>>>>> That
>> > means that this is
>> > >>>> >>>>>> the new webrev with only
>> > >>>> >>>>>> > Fatal
>> > >>>> >>>>>> > >>>>>> and
>> > cleans up the
>> > >>>> tests so
>> > >>>> >>>>>> that it is no longer in
>> > >>>> >>>>>> > >>>>>> between
>> > two worlds:
>> > >>>> >>>>>> > >>>>>>
>> > >>>> >>>>>> > >>>>>> Webrev:
>> > >>>> >>>>>> > >>>>>>
>> > >>>> >>>>>>
>> http://cr.openjdk.java.net/~jcbeyler/8213501/webrev.02/
>> > >>>> >>>>>>
>> > <http://cr.openjdk.java.net/%7Ejcbeyler/8213501/webrev.02/>
>> > >>>> >>>>>> > >>>>>>
>> > >>>> >>>>>>
>> > <http://cr.openjdk.java.net/%7Ejcbeyler/8213501/webrev.02/>
>> > >>>> >>>>>> > >>>>>> Bug:
>> > >>>> >>>>>> >
>> > >>>> https://bugs.openjdk.java.net/browse/JDK-8213501
>> > >>>> >>>>>> > >>>>>>
>> > >>>> >>>>>> > >>>>>> (This
>> > passes testing
>> > >>>> on my
>> > >>>> >>>>>> dev machine for all the
>> > >>>> >>>>>> > >>>>>> modified
>> > tests)
>> > >>>> >>>>>> > >>>>>>
>> > >>>> >>>>>> > >>>>>> with the
>> > example you
>> > >>>> >>>>>> provided, it now looks like:
>> > >>>> >>>>>> > >>>>>>
>> > >>>> >>>>>> >
>> > >>>> >>>>>>
>> > >>>>
>> >
>> http://cr.openjdk.java.net/~jcbeyler/8213501/webrev.02/test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/allocation/AP04/ap04t003/ap04t003.cpp.frames.html
>> > >>>> >>>>>>
>> > >>>> >>>>>> <
>> > >>>>
>> >
>> http://cr.openjdk.java.net/%7Ejcbeyler/8213501/webrev.02/test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/allocation/AP04/ap04t003/ap04t003.cpp.frames.html
>> >
>> > >>>>
>> > >>>> >>>>>>
>> > >>>> >>>>>> >
>> > >>>> >>>>>> > >>>>>>
>> > >>>> >>>>>> > >>>>>>
>> > >>>> >>>>>> >
>> > >>>> >>>>>> <
>> > >>>>
>> >
>> http://cr.openjdk.java.net/%7Ejcbeyler/8213501/webrev.02/test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/allocation/AP04/ap04t003/ap04t003.cpp.frames.html
>> >
>> > >>>>
>> > >>>> >>>>>>
>> > >>>> >>>>>> >
>> > >>>> >>>>>> > >>>>>>
>> > >>>> >>>>>> > >>>>>>
>> > >>>> >>>>>> > >>>>>> Where it
>> > does, to me at
>> > >>>> >>>>>> least, seem cleaner and less
>> > >>>> >>>>>> > >>>>>> "noisy".
>> > >>>> >>>>>> > >>>>>>
>> > >>>> >>>>>> > >>>>>> Let me
>> > know what you
>> > >>>> think,
>> > >>>> >>>>>> > >>>>>> Jc
>> > >>>> >>>>>> > >>>>>>
>> > >>>> >>>>>> > >>>>>>
>> > >>>> >>>>>> > >>>>>> On Tue,
>> > Nov 20, 2018 at
>> > >>>> >>>>>> 9:33 PM Chris Plummer
>> > >>>> >>>>>> > >>>>>> <
>> > >>>> chris.plummer at oracle.com <mailto:chris.plummer at oracle.com
>> >
>> > >>>> >>>>>> <mailto:chris.plummer at oracle.com
>> > <mailto:chris.plummer at oracle.com>>
>> > >>>> >>>>>> > <mailto:
>> chris.plummer at oracle.com
>> > <mailto:chris.plummer at oracle.com>
>> > >>>> >>>>>> <mailto:chris.plummer at oracle.com
>> > <mailto:chris.plummer at oracle.com>>>
>> > >>>> >>>>>> > >>>>>>
>> > <mailto:chris.plummer at oracle.com <mailto:chris.plummer at oracle.com>
>> > >>>> >>>>>> <mailto:chris.plummer at oracle.com
>> > <mailto:chris.plummer at oracle.com>>
>> > >>>> >>>>>> > <mailto:
>> chris.plummer at oracle.com
>> > <mailto:chris.plummer at oracle.com>
>> > >>>> >>>>>> <mailto:chris.plummer at oracle.com
>> > <mailto:chris.plummer at oracle.com>>>>> wrote:
>> > >>>> >>>>>> > >>>>>>
>> > >>>> >>>>>> > >>>>>> Hi
>> JC,
>> > >>>> >>>>>> > >>>>>>
>> > >>>> >>>>>> > >>>>>> Sorry
>> > about the
>> > >>>> delay.
>> > >>>> >>>>>> I had to go back an
>> > >>>> >>>>>> > look at
>> > >>>> >>>>>> > >>>>>> the
>> > initial 8210842
>> > >>>> >>>>>> webrev and RFR thread to see
>> > >>>> >>>>>> > >>>>>> what
>> > this was
>> > >>>> >>>>>> initially all about.
>> > >>>> >>>>>> > >>>>>>
>> > >>>> >>>>>> > >>>>>> In
>> > general the
>> > >>>> changes
>> > >>>> >>>>>> look good.
>> > >>>> >>>>>> > >>>>>>
>> > >>>> >>>>>> > >>>>>> I
>> > don't have a good
>> > >>>> >>>>>> answer to your
>> > >>>> >>>>>> > >>>>>> FatalError/NonFatalError
>> > question. It
>> > >>>> makes
>> > >>>> >>>>>> > the code
>> > >>>> >>>>>> > >>>>>> a lot
>> > cleaner to
>> > >>>> use
>> > >>>> >>>>>> FatalError, but then it
>> > >>>> >>>>>> > is a
>> > >>>> >>>>>> > >>>>>>
>> > behavior change,
>> > >>>> and
>> > >>>> >>>>>> you also need to deal with
>> > >>>> >>>>>> > >>>>>> tests
>> > that
>> > >>>> >>>>>> intentionally induce errors (do
>> > >>>> >>>>>> > you have
>> > >>>> >>>>>> > >>>>>> an
>> > example of
>> > >>>> that).
>> > >>>> >>>>>> > >>>>>>
>> > >>>> >>>>>> > >>>>>> In
>> > any case, right
>> > >>>> now
>> > >>>> >>>>>> your webrev seems to be
>> > >>>> >>>>>> > >>>>>>
>> > between two worlds.
>> > >>>> >>>>>> You are producing
>> > >>>> >>>>>> > FatalError,
>> > >>>> >>>>>> > >>>>>> but
>> > still checking
>> > >>>> >>>>>> results. Here's a good
>> > >>>> >>>>>> > example:
>> > >>>> >>>>>> > >>>>>>
>> > >>>> >>>>>> > >>>>>>
>> > >>>> >>>>>> >
>> > >>>> >>>>>>
>> > >>>>
>> >
>> http://cr.openjdk.java.net/~jcbeyler/8213501/webrev.01/test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/allocation/AP04/ap04t003/ap04t003.cpp.frames.html
>> > >>>> >>>>>>
>> > >>>> >>>>>> <
>> > >>>>
>> >
>> http://cr.openjdk.java.net/%7Ejcbeyler/8213501/webrev.01/test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/allocation/AP04/ap04t003/ap04t003.cpp.frames.html
>> >
>> > >>>>
>> > >>>> >>>>>>
>> > >>>> >>>>>> >
>> > >>>> >>>>>> > >>>>>>
>> > >>>> >>>>>> > >>>>>>
>> > >>>> >>>>>> >
>> > >>>> >>>>>> <
>> > >>>>
>> >
>> http://cr.openjdk.java.net/%7Ejcbeyler/8213501/webrev.01/test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/allocation/AP04/ap04t003/ap04t003.cpp.frames.html
>> >
>> > >>>>
>> > >>>> >>>>>>
>> > >>>> >>>>>> >
>> > >>>> >>>>>> > >>>>>>
>> > >>>> >>>>>> > >>>>>>
>> > >>>> >>>>>> > >>>>>> I'm
>> > not sure if
>> > >>>> this
>> > >>>> >>>>>> is just a temporary
>> > >>>> >>>>>> > state until
>> > >>>> >>>>>> > >>>>>> it
>> > was decided
>> > >>>> which
>> > >>>> >>>>>> approach to take.
>> > >>>> >>>>>> > >>>>>>
>> > >>>> >>>>>> > >>>>>>
>> thanks,
>> > >>>> >>>>>> > >>>>>>
>> > >>>> >>>>>> > >>>>>> Chris
>> > >>>> >>>>>> > >>>>>>
>> > >>>> >>>>>> > >>>>>>
>> > >>>> >>>>>> > >>>>>> On
>> > 11/20/18 2:14
>> > >>>> PM,
>> > >>>> >>>>>> JC Beyler wrote:
>> > >>>> >>>>>> > >>>>>>> Hi
>> all,
>> > >>>> >>>>>> > >>>>>>>
>> > >>>> >>>>>> > >>>>>>>
>> > Chris thought it
>> > >>>> made
>> > >>>> >>>>>> sense to have more
>> > >>>> >>>>>> > eyes on
>> > >>>> >>>>>> > >>>>>>> this
>> > change than
>> > >>>> just
>> > >>>> >>>>>> serviceability as it will
>> > >>>> >>>>>> > >>>>>>>
>> > modify to tests
>> > >>>> that
>> > >>>> >>>>>> are not only
>> > >>>> >>>>>> > serviceability
>> > >>>> >>>>>> > >>>>>>>
>> > tests so I've
>> > >>>> moved
>> > >>>> >>>>>> this to conversation
>> > >>>> >>>>>> > here :)
>> > >>>> >>>>>> > >>>>>>>
>> > >>>> >>>>>> > >>>>>>> For
>> > convenience,
>> > >>>> I've
>> > >>>> >>>>>> copy-pasted the
>> > >>>> >>>>>> > initial RFR:
>> > >>>> >>>>>> > >>>>>>>
>> > >>>> >>>>>> > >>>>>>>
>> > Could I have a
>> > >>>> review
>> > >>>> >>>>>> for the extension and
>> > >>>> >>>>>> > usage
>> > >>>> >>>>>> > >>>>>>> of
>> the
>> > >>>> >>>>>> ExceptionJniWrapper. This adds
>> > lines and
>> > >>>> >>>>>> > >>>>>>>
>> > filenames to the
>> > >>>> end
>> > >>>> >>>>>> of the wrapper JNI
>> > >>>> >>>>>> > methods,
>> > >>>> >>>>>> > >>>>>>> adds
>> > tracing, and
>> > >>>> >>>>>> throws an error if need
>> > >>>> >>>>>> > be. I've
>> > >>>> >>>>>> > >>>>>>>
>> > ported the gc/lock
>> > >>>> >>>>>> files to use the new
>> > >>>> >>>>>> > >>>>>>>
>> > TRACE_JNI_CALL
>> > >>>> add-on
>> > >>>> >>>>>> and I've ported a few
>> > >>>> >>>>>> > of the
>> > >>>> >>>>>> > >>>>>>>
>> > tests that were
>> > >>>> >>>>>> already changed for the
>> > >>>> >>>>>> > assignment
>> > >>>> >>>>>> > >>>>>>>
>> > webrev for
>> > >>>> >>>>>> JDK-8212884.
>> > >>>> >>>>>> > >>>>>>>
>> > >>>> >>>>>> > >>>>>>>
>> Webrev:
>> > >>>> >>>>>> > >>>>>>>
>> > >>>> >>>>>>
>> http://cr.openjdk.java.net/~jcbeyler/8213501/webrev.01
>> > >>>> >>>>>>
>> > <http://cr.openjdk.java.net/%7Ejcbeyler/8213501/webrev.01>
>> > >>>> >>>>>> > >>>>>>>
>> > >>>> >>>>>>
>> > <http://cr.openjdk.java.net/%7Ejcbeyler/8213501/webrev.01>
>> > >>>> >>>>>> > >>>>>>> Bug:
>> > >>>> >>>>>> > >>>>>>>
>> > >>>> >>>>>> https://bugs.openjdk.java.net/browse/JDK-8213501
>> > >>>> >>>>>> > >>>>>>>
>> > >>>> >>>>>> > >>>>>>> For
>> > illustration,
>> > >>>> if
>> > >>>> >>>>>> I force an error to the
>> > >>>> >>>>>> > >>>>>>>
>> > AP04/ap04t03 test
>> > >>>> and
>> > >>>> >>>>>> set the verbosity on,
>> > >>>> >>>>>> > I get
>> > >>>> >>>>>> > >>>>>>>
>> > something like:
>> > >>>> >>>>>> > >>>>>>>
>> > >>>> >>>>>> > >>>>>>> >>
>> > Calling JNI
>> > >>>> method
>> > >>>> >>>>>> FindClass from
>> > >>>> >>>>>> > >>>>>>> ap04t003.cpp:343
>> > >>>> >>>>>> > >>>>>>> >>
>> > Calling with
>> > >>>> these
>> > >>>> >>>>>> parameter(s):
>> > >>>> >>>>>> > >>>>>>> java/lang/Threadd
>> > >>>> >>>>>> > >>>>>>> Wait
>> > for thread
>> > >>>> to
>> > >>>> >>>>>> finish
>> > >>>> >>>>>> > >>>>>>> <<
>> > Called JNI
>> > >>>> method
>> > >>>> >>>>>> FindClass from
>> > >>>> >>>>>> > >>>>>>> ap04t003.cpp:343
>> > >>>> >>>>>> > >>>>>>>
>> > Exception in
>> > >>>> thread
>> > >>>> >>>>>> "Thread-0"
>> > >>>> >>>>>> > >>>>>>>
>> > java.lang.NoClassDefFoundError:
>> > >>>> >>>>>> > java/lang/Threadd
>> > >>>> >>>>>> > >>>>>>>
>> > at
>> > >>>> >>>>>> > >>>>>>>
>> > >>>> >>>>>> >
>> > >>>> >>>>>>
>> > >>>>
>> >
>> nsk.jvmti.scenarios.allocation.AP04.ap04t003.runIterateOverHeap(Native
>> > >>>> >>>>>>
>> > >>>> >>>>>> > >>>>>>>
>> > >>>> >>>>>> > >>>>>>>
>> Method)
>> > >>>> >>>>>> > >>>>>>>
>> > at
>> > >>>> >>>>>> > >>>>>>>
>> > >>>> >>>>>> >
>> > >>>> >>>>>>
>> > >>>>
>> >
>> nsk.jvmti.scenarios.allocation.AP04.ap04t003HeapIterator.runIteration(ap04t003.java:140)
>> > >>>>
>> > >>>> >>>>>>
>> > >>>> >>>>>> >
>> > >>>> >>>>>> > >>>>>>>
>> > >>>> >>>>>> > >>>>>>>
>> > at
>> > >>>> >>>>>> > >>>>>>>
>> > >>>> >>>>>> >
>> > >>>> >>>>>>
>> > >>>>
>> >
>> nsk.jvmti.scenarios.allocation.AP04.ap04t003Thread.run(ap04t003.java:201)
>> > >>>> >>>>>>
>> > >>>> >>>>>> >
>> > >>>> >>>>>> > >>>>>>>
>> > >>>> >>>>>> > >>>>>>>
>> > Caused by:
>> > >>>> >>>>>> java.lang.ClassNotFoundException:
>> > >>>> >>>>>> > >>>>>>>
>> > java.lang.Threadd
>> > >>>> >>>>>> > >>>>>>>
>> > at
>> > >>>> >>>>>> > >>>>>>>
>> > >>>> >>>>>> >
>> > >>>> >>>>>>
>> > >>>>
>> >
>> java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:583)
>> > >>>>
>> > >>>> >>>>>>
>> > >>>> >>>>>> >
>> > >>>> >>>>>> > >>>>>>>
>> > >>>> >>>>>> > >>>>>>>
>> > at
>> > >>>> >>>>>> > >>>>>>>
>> > >>>> >>>>>> >
>> > >>>> >>>>>>
>> > >>>>
>> >
>> java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:178)
>> > >>>>
>> > >>>> >>>>>>
>> > >>>> >>>>>> >
>> > >>>> >>>>>> > >>>>>>>
>> > >>>> >>>>>> > >>>>>>>
>> > at
>> > >>>> >>>>>> > >>>>>>>
>> > >>>> >>>>>>
>> > java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:521)
>> > >>>> >>>>>> > >>>>>>>
>> > ... 3 more
>> > >>>> >>>>>> > >>>>>>>
>> > FATAL ERROR in
>> > >>>> native
>> > >>>> >>>>>> method: JNI method
>> > >>>> >>>>>> > FindClass
>> > >>>> >>>>>> > >>>>>>> :
>> > internal error
>> > >>>> from
>> > >>>> >>>>>> ap04t003.cpp:343
>> > >>>> >>>>>> > >>>>>>>
>> > at
>> > >>>> >>>>>> > >>>>>>>
>> > >>>> >>>>>> >
>> > >>>> >>>>>>
>> > >>>>
>> >
>> nsk.jvmti.scenarios.allocation.AP04.ap04t003.runIterateOverHeap(Native
>> > >>>> >>>>>>
>> > >>>> >>>>>> > >>>>>>>
>> > >>>> >>>>>> > >>>>>>>
>> Method)
>> > >>>> >>>>>> > >>>>>>>
>> > at
>> > >>>> >>>>>> > >>>>>>>
>> > >>>> >>>>>> >
>> > >>>> >>>>>>
>> > >>>>
>> >
>> nsk.jvmti.scenarios.allocation.AP04.ap04t003HeapIterator.runIteration(ap04t003.java:140)
>> > >>>>
>> > >>>> >>>>>>
>> > >>>> >>>>>> >
>> > >>>> >>>>>> > >>>>>>>
>> > >>>> >>>>>> > >>>>>>>
>> > at
>> > >>>> >>>>>> > >>>>>>>
>> > >>>> >>>>>> >
>> > >>>> >>>>>>
>> > >>>>
>> >
>> nsk.jvmti.scenarios.allocation.AP04.ap04t003Thread.run(ap04t003.java:201)
>> > >>>> >>>>>>
>> > >>>> >>>>>> >
>> > >>>> >>>>>> > >>>>>>>
>> > >>>> >>>>>> > >>>>>>>
>> > >>>> >>>>>> > >>>>>>>
>> > >>>> Questions/comments I
>> > >>>> >>>>>> have about this are:
>> > >>>> >>>>>> > >>>>>>> -
>> > Do we want to
>> > >>>> >>>>>> force fatal errors when a JNI
>> > >>>> >>>>>> > >>>>>>> call
>> > fails in
>> > >>>> >>>>>> general? Most of these tests
>> > >>>> >>>>>> > do the
>> > >>>> >>>>>> > >>>>>>>
>> > right thing and
>> > >>>> test
>> > >>>> >>>>>> the return of the JNI
>> > >>>> >>>>>> > calls,
>> > >>>> >>>>>> > >>>>>>> for
>> > example:
>> > >>>> >>>>>> > >>>>>>>
>> > thrClass =
>> > >>>> >>>>>> >
>> jni->FindClass("java/lang/Threadd",
>> > >>>> >>>>>> > >>>>>>>
>> > TRACE_JNI_CALL);
>> > >>>> >>>>>> > >>>>>>>
>> > if (thrClass
>> > >>>> ==
>> > >>>> >>>>>> NULL) {
>> > >>>> >>>>>> > >>>>>>>
>> > >>>> >>>>>> > >>>>>>> but
>> > now the
>> > >>>> wrapper
>> > >>>> >>>>>> actually would do a
>> > >>>> >>>>>> > fatal if
>> > >>>> >>>>>> > >>>>>>> the
>> > FindClass call
>> > >>>> >>>>>> would return a nullptr,
>> > >>>> >>>>>> > so we
>> > >>>> >>>>>> > >>>>>>>
>> > could remove that
>> > >>>> >>>>>> test altogether. What do you
>> > >>>> >>>>>> > >>>>>>> think?
>> > >>>> >>>>>> > >>>>>>>
>> > - I prefer to
>> > >>>> >>>>>> leave them as the tests then
>> > >>>> >>>>>> > >>>>>>>
>> > become closer to
>> > >>>> what
>> > >>>> >>>>>> real users would have in
>> > >>>> >>>>>> > >>>>>>>
>> > their code and is
>> > >>>> the
>> > >>>> >>>>>> "recommended" way of
>> > >>>> >>>>>> > doing it
>> > >>>> >>>>>> > >>>>>>>
>> > >>>> >>>>>> > >>>>>>>
>> - The
>> > >>>> alternative
>> > >>>> >>>>>> is to use the
>> > >>>> >>>>>> > NonFatalError I
>> > >>>> >>>>>> > >>>>>>>
>> > added which then
>> > >>>> just
>> > >>>> >>>>>> prints out that something
>> > >>>> >>>>>> > >>>>>>> went
>> > wrong,
>> > >>>> letting
>> > >>>> >>>>>> the test continue. Question
>> > >>>> >>>>>> > >>>>>>> will
>> > be what
>> > >>>> should
>> > >>>> >>>>>> be the default? The
>> > >>>> >>>>>> > fatal or
>> > >>>> >>>>>> > >>>>>>> the
>> > non-fatal
>> > >>>> error
>> > >>>> >>>>>> handling?
>> > >>>> >>>>>> > >>>>>>>
>> > >>>> >>>>>> > >>>>>>> On a
>> > different
>> > >>>> >>>>>> subject:
>> > >>>> >>>>>> > >>>>>>> -
>> > On the new
>> > >>>> tests,
>> > >>>> >>>>>> I've removed the
>> > >>>> >>>>>> > >>>>>>>
>> > NSK_JNI_VERIFY
>> > >>>> since
>> > >>>> >>>>>> the JNI wrapper
>> > >>>> >>>>>> > handles the
>> > >>>> >>>>>> > >>>>>>>
>> > tracing and the
>> > >>>> >>>>>> verify in almost the same
>> > >>>> >>>>>> > way; only
>> > >>>> >>>>>> > >>>>>>>
>> > difference I can
>> > >>>> >>>>>> really tell is that the
>> > >>>> >>>>>> > complain
>> > >>>> >>>>>> > >>>>>>>
>> > method from NSK
>> > >>>> has a
>> > >>>> >>>>>> max complain before
>> > >>>> >>>>>> > stopping
>> > >>>> >>>>>> > >>>>>>> to
>> > "complain"; I
>> > >>>> have
>> > >>>> >>>>>> not added that part
>> > >>>> >>>>>> > of the
>> > >>>> >>>>>> > >>>>>>> code
>> > in this
>> > >>>> webrev
>> > >>>> >>>>>> > >>>>>>>
>> > >>>> >>>>>> > >>>>>>> Once
>> > we decide on
>> > >>>> >>>>>> these, I can continue on the
>> > >>>> >>>>>> > >>>>>>>
>> > files from
>> > >>>> >>>>>> JDK-8212884 and then do both the
>> > >>>> >>>>>> > >>>>>>>
>> > assignment in an
>> > >>>> if
>> > >>>> >>>>>> extraction followed-by this
>> > >>>> >>>>>> > >>>>>>> type
>> > of webrev in
>> > >>>> an
>> > >>>> >>>>>> easier fashion.
>> > >>>> >>>>>> > Depending on
>> > >>>> >>>>>> > >>>>>>>
>> > decisions here,
>> > >>>> >>>>>> NSK*VERIFY can be deprecated as
>> > >>>> >>>>>> > >>>>>>> well
>> > as we go
>> > >>>> forward.
>> > >>>> >>>>>> > >>>>>>>
>> > >>>> >>>>>> > >>>>>>>
>> Thanks!
>> > >>>> >>>>>> > >>>>>>> Jc
>> > >>>> >>>>>> > >>>>>>>
>> > >>>> >>>>>> > >>>>>>> On
>> > Mon, Nov 19,
>> > >>>> 2018
>> > >>>> >>>>>> at 11:34 AM Chris Plummer
>> > >>>> >>>>>> > >>>>>>> <
>> > >>>> chris.plummer at oracle.com <mailto:chris.plummer at oracle.com
>> >
>> > >>>> >>>>>> <mailto:chris.plummer at oracle.com
>> > <mailto:chris.plummer at oracle.com>>
>> > >>>> >>>>>> > <mailto:
>> chris.plummer at oracle.com
>> > <mailto:chris.plummer at oracle.com>
>> > >>>> >>>>>> <mailto:chris.plummer at oracle.com
>> > <mailto:chris.plummer at oracle.com>>>
>> > >>>> >>>>>> > >>>>>>>
>> > <mailto:chris.plummer at oracle.com <mailto:chris.plummer at oracle.com>
>> > >>>> >>>>>> <mailto:chris.plummer at oracle.com
>> > <mailto:chris.plummer at oracle.com>>
>> > >>>> >>>>>> > <mailto:
>> chris.plummer at oracle.com
>> > <mailto:chris.plummer at oracle.com>
>> > >>>> >>>>>> <mailto:chris.plummer at oracle.com
>> > <mailto:chris.plummer at oracle.com>>>>> wrote:
>> > >>>> >>>>>> > >>>>>>>
>> > >>>> >>>>>> > >>>>>>>
>> > On 11/19/18
>> > >>>> 10:07
>> > >>>> >>>>>> AM, JC Beyler wrote:
>> > >>>> >>>>>> > >>>>>>>>
>> > Hi all,
>> > >>>> >>>>>> > >>>>>>>>
>> > >>>> >>>>>> > >>>>>>>>
>> > @David/Chris:
>> > >>>> >>>>>> should I then push this
>> > >>>> >>>>>> > RFR to
>> > >>>> >>>>>> > >>>>>>>>
>> > the hotspot
>> > >>>> >>>>>> mailing or the runtime
>> > >>>> >>>>>> > one? For
>> > >>>> >>>>>> > >>>>>>>>
>> > what it's
>> > >>>> worth,
>> > >>>> >>>>>> a lot of the tests
>> > >>>> >>>>>> > under the
>> > >>>> >>>>>> > >>>>>>>>
>> > vmTestbase
>> > >>>> are
>> > >>>> >>>>>> jvmti so the review also
>> > >>>> >>>>>> > >>>>>>>>
>> > affects
>> > >>>> >>>>>> serviceability; it just turns
>> > >>>> >>>>>> > out I
>> > >>>> >>>>>> > >>>>>>>>
>> > started with
>> > >>>> the
>> > >>>> >>>>>> GC originally and
>> > >>>> >>>>>> > then hit
>> > >>>> >>>>>> > >>>>>>>>
>> > some other
>> > >>>> tests
>> > >>>> >>>>>> I had touched via the
>> > >>>> >>>>>> > >>>>>>>>
>> > assignment
>> > >>>> >>>>>> extraction.
>> > >>>> >>>>>> > >>>>>>>
>> > I think
>> > >>>> hotspot
>> > >>>> >>>>>> would be best.
>> > >>>> >>>>>> > >>>>>>>
>> > >>>> >>>>>> > >>>>>>>
>> > Chris
>> > >>>> >>>>>> > >>>>>>>>
>> > >>>> >>>>>> > >>>>>>>>
>> > @Serguei:
>> > >>>> Done
>> > >>>> >>>>>> for the method
>> > >>>> >>>>>> > renaming, for
>> > >>>> >>>>>> > >>>>>>>>
>> > the indent,
>> > >>>> are
>> > >>>> >>>>>> you talking about
>> > >>>> >>>>>> > going from
>> > >>>> >>>>>> > >>>>>>>>
>> > the 8-indent
>> > >>>> to
>> > >>>> >>>>>> 4-indent? If so, would
>> > >>>> >>>>>> > it not
>> > >>>> >>>>>> > >>>>>>>>
>> > just be
>> > >>>> better
>> > >>>> >>>>>> to do a new JBS bug and
>> > >>>> >>>>>> > do the
>> > >>>> >>>>>> > >>>>>>>>
>> > whole files
>> > >>>> in
>> > >>>> >>>>>> one go? I ask because
>> > >>>> >>>>>> > >>>>>>>>
>> > otherwise, it
>> > >>>> >>>>>> will look a bit weird to
>> > >>>> >>>>>> > have
>> > >>>> >>>>>> > >>>>>>>>
>> > parts of the
>> > >>>> >>>>>> file as 8-indent and others
>> > >>>> >>>>>> > >>>>>>>> 4-indent?
>> > >>>> >>>>>> > >>>>>>>>
>> > >>>> >>>>>> > >>>>>>>>
>> > Thanks for
>> > >>>> >>>>>> looking at it!
>> > >>>> >>>>>> > >>>>>>>>
>> Jc
>> > >>>> >>>>>> > >>>>>>>>
>> > >>>> >>>>>> > >>>>>>>>
>> > On Mon, Nov
>> > >>>> 19,
>> > >>>> >>>>>> 2018 at 1:25 AM
>> > >>>> >>>>>> > >>>>>>>>
>> > serguei.spitsyn at oracle.com <mailto:serguei.spitsyn at oracle.com>
>> > >>>> >>>>>> <mailto:serguei.spitsyn at oracle.com
>> > <mailto:serguei.spitsyn at oracle.com>>
>> > >>>> >>>>>> <mailto:serguei.spitsyn at oracle.com
>> > <mailto:serguei.spitsyn at oracle.com>
>> > >>>> >>>>>> <mailto:serguei.spitsyn at oracle.com
>> > <mailto:serguei.spitsyn at oracle.com>>>
>> > >>>> >>>>>> > >>>>>>>> <mailto:
>> > >>>> serguei.spitsyn at oracle.com <mailto:
>> serguei.spitsyn at oracle.com>
>> > >>>> >>>>>> <mailto:serguei.spitsyn at oracle.com
>> > <mailto:serguei.spitsyn at oracle.com>>
>> > >>>> >>>>>> >
>> > <mailto:serguei.spitsyn at oracle.com <mailto:
>> serguei.spitsyn at oracle.com>
>> > >>>> >>>>>> <mailto:serguei.spitsyn at oracle.com
>> > <mailto:serguei.spitsyn at oracle.com>>>>
>> > >>>> >>>>>> > >>>>>>>>
>> > <serguei.spitsyn at oracle.com <mailto:serguei.spitsyn at oracle.com>
>> > >>>> >>>>>> <mailto:serguei.spitsyn at oracle.com
>> > <mailto:serguei.spitsyn at oracle.com>>
>> > >>>> >>>>>> >
>> > <mailto:serguei.spitsyn at oracle.com <mailto:
>> serguei.spitsyn at oracle.com>
>> > >>>> >>>>>> <mailto:serguei.spitsyn at oracle.com
>> > <mailto:serguei.spitsyn at oracle.com>>>
>> > >>>> >>>>>> > >>>>>>>> <mailto:
>> > >>>> serguei.spitsyn at oracle.com <mailto:
>> serguei.spitsyn at oracle.com>
>> > >>>> >>>>>> <mailto:serguei.spitsyn at oracle.com
>> > <mailto:serguei.spitsyn at oracle.com>>
>> > >>>> >>>>>> >
>> > <mailto:serguei.spitsyn at oracle.com <mailto:
>> serguei.spitsyn at oracle.com>
>> > >>>> >>>>>> <mailto:serguei.spitsyn at oracle.com
>> > <mailto:serguei.spitsyn at oracle.com>>>>> wrote:
>> > >>>> >>>>>> > >>>>>>>>
>> > >>>> >>>>>> > >>>>>>>>
>> > Hi Jc,
>> > >>>> >>>>>> > >>>>>>>>
>> > >>>> >>>>>> > >>>>>>>>
>> > We have
>> > >>>> to
>> > >>>> >>>>>> start this review
>> > >>>> >>>>>> > anyway. :)
>> > >>>> >>>>>> > >>>>>>>>
>> > It looks
>> > >>>> >>>>>> good to me in general.
>> > >>>> >>>>>> > >>>>>>>>
>> > Thank you
>> > >>>> >>>>>> for your consistency in this
>> > >>>> >>>>>> > >>>>>>>>
>> > >>>> refactoring!
>> > >>>> >>>>>> > >>>>>>>>
>> > >>>> >>>>>> > >>>>>>>>
>> > Some
>> > >>>> minor
>> > >>>> >>>>>> comments.
>> > >>>> >>>>>> > >>>>>>>>
>> > >>>> >>>>>> > >>>>>>>>
>> > >>>> >>>>>> >
>> > >>>> >>>>>>
>> > >>>>
>> >
>> http://cr.openjdk.java.net/%7Ejcbeyler/8213501/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/share/jni/ExceptionCheckingJniEnv.cpp.udiff.html
>> > >>>> >>>>>>
>> > >>>> >>>>>> >
>> > >>>> >>>>>> > >>>>>>>>
>> > >>>> >>>>>> > >>>>>>>>
>> > +static
>> > >>>> >>>>>> const char*
>> > >>>> >>>>>> > remove_folders(const
>> > >>>> >>>>>> > >>>>>>>>
>> > char*
>> > >>>> >>>>>> fullname) { I'd suggest to
>> > >>>> >>>>>> > rename
>> > >>>> >>>>>> > >>>>>>>>
>> > the
>> > >>>> function
>> > >>>> >>>>>> name to something
>> > >>>> >>>>>> > traditional
>> > >>>> >>>>>> > >>>>>>>>
>> > like
>> > >>>> >>>>>> get_basename. Otherwise, it
>> > >>>> >>>>>> > sounds
>> > >>>> >>>>>> > >>>>>>>>
>> > like this
>> > >>>> >>>>>> function has to really
>> > >>>> >>>>>> > remove
>> > >>>> >>>>>> > >>>>>>>>
>> > folders.
>> > >>>> :)
>> > >>>> >>>>>> Also, all *Locker.cpp have
>> > >>>> >>>>>> > >>>>>>>>
>> > wrong
>> > >>>> indent
>> > >>>> >>>>>> in the bodies of if
>> > >>>> >>>>>> > and while
>> > >>>> >>>>>> > >>>>>>>>
>> > >>>> statements.
>> > >>>> >>>>>> Could this be fixed
>> > >>>> >>>>>> > with the
>> > >>>> >>>>>> > >>>>>>>>
>> > >>>> refactoring?
>> > >>>> >>>>>> I did not look on how
>> > >>>> >>>>>> > this
>> > >>>> >>>>>> > >>>>>>>>
>> > impacts
>> > >>>> the
>> > >>>> >>>>>> tests other than
>> > >>>> >>>>>> > >>>>>>>>
>> > serviceability.
>> > >>>> Thanks,
>> > >>>> >>>>>> Serguei
>> > >>>> >>>>>> > >>>>>>>>
>> > >>>> >>>>>> > >>>>>>>>
>> > >>>> >>>>>> > >>>>>>>>
>> > On
>> > >>>> 11/16/18
>> > >>>> >>>>>> 19:43, JC Beyler wrote:
>> > >>>> >>>>>> > >>>>>>>>>
>> > Hi all,
>> > >>>> >>>>>> > >>>>>>>>>
>> > >>>> >>>>>> > >>>>>>>>>
>> > Anybody
>> > >>>> >>>>>> motivated to review this? :)
>> > >>>> >>>>>> > >>>>>>>>>
>> Jc
>> > >>>> >>>>>> > >>>>>>>>>
>> > >>>> >>>>>> > >>>>>>>>>
>> > On Wed, Nov
>> > >>>> 7,
>> > >>>> >>>>>> 2018 at 9:53 PM JC
>> > >>>> >>>>>> > Beyler
>> > >>>> >>>>>> > >>>>>>>>>
>> > <jcbeyler at google.com <mailto:jcbeyler at google.com>
>> > >>>> >>>>>> <mailto:jcbeyler at google.com
>> > <mailto:jcbeyler at google.com>>
>> > >>>> >>>>>> > <mailto:jcbeyler at google.com
>> > <mailto:jcbeyler at google.com>
>> > >>>> >>>>>> <mailto:jcbeyler at google.com
>> > <mailto:jcbeyler at google.com>>>
>> > >>>> >>>>>> > >>>>>>>>>
>> > <mailto:jcbeyler at google.com <mailto:jcbeyler at google.com>
>> > >>>> >>>>>> <mailto:jcbeyler at google.com
>> > <mailto:jcbeyler at google.com>>
>> > >>>> >>>>>> > <mailto:jcbeyler at google.com
>> > <mailto:jcbeyler at google.com>
>> > >>>> >>>>>> <mailto:jcbeyler at google.com
>> > <mailto:jcbeyler at google.com>>>>> wrote:
>> > >>>> >>>>>> > >>>>>>>>>
>> > >>>> >>>>>> > >>>>>>>>>
>> > Hi all,
>> > >>>> >>>>>> > >>>>>>>>>
>> > >>>> >>>>>> > >>>>>>>>>
>> > Could I
>> > >>>> have
>> > >>>> >>>>>> a review for the
>> > >>>> >>>>>> > >>>>>>>>>
>> > extension
>> > >>>> >>>>>> and usage of the
>> > >>>> >>>>>> > >>>>>>>>> ExceptionJniWrapper.
>> This
>> > >>>> >>>>>> > adds lines
>> > >>>> >>>>>> > >>>>>>>>>
>> > and
>> > >>>> >>>>>> filenames to the end of the
>> > >>>> >>>>>> > >>>>>>>>>
>> > wrapper
>> > >>>> JNI
>> > >>>> >>>>>> methods, adds
>> > >>>> >>>>>> > tracing,
>> > >>>> >>>>>> > >>>>>>>>>
>> > and
>> > >>>> throws
>> > >>>> >>>>>> an error if need
>> > >>>> >>>>>> > be. I've
>> > >>>> >>>>>> > >>>>>>>>>
>> > ported
>> > >>>> the
>> > >>>> >>>>>> gc/lock files to
>> > >>>> >>>>>> > use the
>> > >>>> >>>>>> > >>>>>>>>>
>> > new
>> > >>>> >>>>>> TRACE_JNI_CALL add-on and
>> > >>>> >>>>>> > I've
>> > >>>> >>>>>> > >>>>>>>>>
>> > ported a
>> > >>>> few
>> > >>>> >>>>>> of the tests
>> > >>>> >>>>>> > that were
>> > >>>> >>>>>> > >>>>>>>>>
>> > already
>> > >>>> >>>>>> changed for the
>> > >>>> >>>>>> > assignment
>> > >>>> >>>>>> > >>>>>>>>>
>> > webrev
>> > >>>> for
>> > >>>> >>>>>> JDK-8212884.
>> > >>>> >>>>>> > >>>>>>>>>
>> > >>>> >>>>>> > >>>>>>>>>
>> > Webrev:
>> > >>>> >>>>>> > >>>>>>>>>
>> > >>>> >>>>>>
>> http://cr.openjdk.java.net/~jcbeyler/8213501/webrev.00/
>> > >>>> >>>>>>
>> > <http://cr.openjdk.java.net/%7Ejcbeyler/8213501/webrev.00/>
>> > >>>> >>>>>> > >>>>>>>>>
>> > >>>> >>>>>>
>> > <http://cr.openjdk.java.net/%7Ejcbeyler/8213501/webrev.00/>
>> > >>>> >>>>>> > >>>>>>>>>
>> > Bug:
>> > >>>> >>>>>> > >>>>>>>>>
>> > >>>> >>>>>> https://bugs.openjdk.java.net/browse/JDK-8213501
>> > >>>> >>>>>> > >>>>>>>>>
>> > >>>> >>>>>> > >>>>>>>>>
>> > For
>> > >>>> >>>>>> illustration, if I force
>> > >>>> >>>>>> > an error
>> > >>>> >>>>>> > >>>>>>>>>
>> > to the
>> > >>>> >>>>>> AP04/ap04t03 test and
>> > >>>> >>>>>> > set the
>> > >>>> >>>>>> > >>>>>>>>>
>> > verbosity
>> > >>>> >>>>>> on, I get something
>> > >>>> >>>>>> > like:
>> > >>>> >>>>>> > >>>>>>>>>
>> > >>>> >>>>>> > >>>>>>>>>
>> > >>
>> > >>>> Calling
>> > >>>> >>>>>> JNI method
>> > >>>> >>>>>> > FindClass from
>> > >>>> >>>>>> > >>>>>>>>> ap04t003.cpp:343
>> > >>>> >>>>>> > >>>>>>>>>
>> > >>
>> > >>>> Calling
>> > >>>> >>>>>> with these
>> > >>>> >>>>>> > parameter(s):
>> > >>>> >>>>>> > >>>>>>>>> java/lang/Threadd
>> > >>>> >>>>>> > >>>>>>>>>
>> > Wait for
>> > >>>> >>>>>> thread to finish
>> > >>>> >>>>>> > >>>>>>>>>
>> > << Called
>> > >>>> >>>>>> JNI method
>> > >>>> >>>>>> > FindClass from
>> > >>>> >>>>>> > >>>>>>>>> ap04t003.cpp:343
>> > >>>> >>>>>> > >>>>>>>>>
>> > >>>> Exception in
>> > >>>> >>>>>> thread "Thread-0"
>> > >>>> >>>>>> > >>>>>>>>>
>> > java.lang.NoClassDefFoundError:
>> > >>>> >>>>>> > >>>>>>>>> java/lang/Threadd
>> > >>>> >>>>>> > >>>>>>>>>
>> > at
>> > >>>> >>>>>> > >>>>>>>>>
>> > >>>> >>>>>> >
>> > >>>> >>>>>>
>> > >>>>
>> >
>> nsk.jvmti.scenarios.allocation.AP04.ap04t003.runIterateOverHeap(Native
>> > >>>> >>>>>>
>> > >>>> >>>>>> > >>>>>>>>>
>> > >>>> >>>>>> > >>>>>>>>>
>> > Method)
>> > >>>> >>>>>> > >>>>>>>>>
>> > at
>> > >>>> >>>>>> > >>>>>>>>>
>> > >>>> >>>>>> >
>> > >>>> >>>>>>
>> > >>>>
>> >
>> nsk.jvmti.scenarios.allocation.AP04.ap04t003HeapIterator.runIteration(ap04t003.java:140)
>> > >>>>
>> > >>>> >>>>>>
>> > >>>> >>>>>> >
>> > >>>> >>>>>> > >>>>>>>>>
>> > >>>> >>>>>> > >>>>>>>>>
>> > at
>> > >>>> >>>>>> > >>>>>>>>>
>> > >>>> >>>>>> >
>> > >>>> >>>>>>
>> > >>>>
>> >
>> nsk.jvmti.scenarios.allocation.AP04.ap04t003Thread.run(ap04t003.java:201)
>> > >>>> >>>>>>
>> > >>>> >>>>>> >
>> > >>>> >>>>>> > >>>>>>>>>
>> > >>>> >>>>>> > >>>>>>>>>
>> > Caused
>> > >>>> by:
>> > >>>> >>>>>> > >>>>>>>>>
>> > java.lang.ClassNotFoundException:
>> > >>>> >>>>>> > >>>>>>>>> java.lang.Threadd
>> > >>>> >>>>>> > >>>>>>>>>
>> > at
>> > >>>> >>>>>> > >>>>>>>>>
>> > >>>> >>>>>> >
>> > >>>> >>>>>>
>> > >>>>
>> >
>> java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:583)
>> > >>>>
>> > >>>> >>>>>>
>> > >>>> >>>>>> >
>> > >>>> >>>>>> > >>>>>>>>>
>> > >>>> >>>>>> > >>>>>>>>>
>> > at
>> > >>>> >>>>>> > >>>>>>>>>
>> > >>>> >>>>>> >
>> > >>>> >>>>>>
>> > >>>>
>> >
>> java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:178)
>> > >>>>
>> > >>>> >>>>>>
>> > >>>> >>>>>> >
>> > >>>> >>>>>> > >>>>>>>>>
>> > >>>> >>>>>> > >>>>>>>>>
>> > at
>> > >>>> >>>>>> > >>>>>>>>>
>> > >>>> >>>>>> >
>> > >>>> >>>>>>
>> > java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:521)
>> > >>>> >>>>>> > >>>>>>>>>
>> > ... 3
>> > >>>> more
>> > >>>> >>>>>> > >>>>>>>>>
>> > FATAL
>> > >>>> ERROR
>> > >>>> >>>>>> in native method: JNI
>> > >>>> >>>>>> > >>>>>>>>>
>> > method
>> > >>>> >>>>>> FindClass : internal error
>> > >>>> >>>>>> > >>>>>>>>>
>> > from
>> > >>>> >>>>>> ap04t003.cpp:343
>> > >>>> >>>>>> > >>>>>>>>>
>> > at
>> > >>>> >>>>>> > >>>>>>>>>
>> > >>>> >>>>>> >
>> > >>>> >>>>>>
>> > >>>>
>> >
>> nsk.jvmti.scenarios.allocation.AP04.ap04t003.runIterateOverHeap(Native
>> > >>>> >>>>>>
>> > >>>> >>>>>> > >>>>>>>>>
>> > >>>> >>>>>> > >>>>>>>>>
>> > Method)
>> > >>>> >>>>>> > >>>>>>>>>
>> > at
>> > >>>> >>>>>> > >>>>>>>>>
>> > >>>> >>>>>> >
>> > >>>> >>>>>>
>> > >>>>
>> >
>> nsk.jvmti.scenarios.allocation.AP04.ap04t003HeapIterator.runIteration(ap04t003.java:140)
>> > >>>>
>> > >>>> >>>>>>
>> > >>>> >>>>>> >
>> > >>>> >>>>>> > >>>>>>>>>
>> > >>>> >>>>>> > >>>>>>>>>
>> > at
>> > >>>> >>>>>> > >>>>>>>>>
>> > >>>> >>>>>> >
>> > >>>> >>>>>>
>> > >>>>
>> >
>> nsk.jvmti.scenarios.allocation.AP04.ap04t003Thread.run(ap04t003.java:201)
>> > >>>> >>>>>>
>> > >>>> >>>>>> >
>> > >>>> >>>>>> > >>>>>>>>>
>> > >>>> >>>>>> > >>>>>>>>>
>> > >>>> >>>>>> > >>>>>>>>> Questions/comments I
>> > have about
>> > >>>> >>>>>> > >>>>>>>>> this are:
>> > >>>> >>>>>> > >>>>>>>>>
>> > - Do we
>> > >>>> >>>>>> want to force fatal
>> > >>>> >>>>>> > errors
>> > >>>> >>>>>> > >>>>>>>>>
>> > when a
>> > >>>> JNI
>> > >>>> >>>>>> call fails in general?
>> > >>>> >>>>>> > >>>>>>>>>
>> > Most of
>> > >>>> >>>>>> these tests do the right
>> > >>>> >>>>>> > >>>>>>>>>
>> > thing and
>> > >>>> >>>>>> test the return of
>> > >>>> >>>>>> > the JNI
>> > >>>> >>>>>> > >>>>>>>>>
>> > calls,
>> > >>>> for
>> > >>>> >>>>>> example:
>> > >>>> >>>>>> > >>>>>>>>>
>> > thrClass
>> > >>>> =
>> > >>>> >>>>>> > >>>>>>>>>
>> > jni->FindClass("java/lang/Threadd",
>> > >>>> >>>>>> > >>>>>>>>> TRACE_JNI_CALL);
>> > >>>> >>>>>> > >>>>>>>>>
>> > if
>> > >>>> >>>>>> (thrClass == NULL) {
>> > >>>> >>>>>> > >>>>>>>>>
>> > >>>> >>>>>> > >>>>>>>>>
>> > but now
>> > >>>> the
>> > >>>> >>>>>> wrapper actually
>> > >>>> >>>>>> > would do
>> > >>>> >>>>>> > >>>>>>>>>
>> > a fatal
>> > >>>> if
>> > >>>> >>>>>> the FindClass call
>> > >>>> >>>>>> > would
>> > >>>> >>>>>> > >>>>>>>>>
>> > return a
>> > >>>> >>>>>> nullptr, so we could
>> > >>>> >>>>>> > remove
>> > >>>> >>>>>> > >>>>>>>>>
>> > that test
>> > >>>> >>>>>> altogether. What do
>> > >>>> >>>>>> > you
>> > >>>> >>>>>> > >>>>>>>>> think?
>> > >>>> >>>>>> > >>>>>>>>>
>> > - I
>> > >>>> >>>>>> prefer to leave them
>> > >>>> >>>>>> > as the
>> > >>>> >>>>>> > >>>>>>>>>
>> > tests
>> > >>>> then
>> > >>>> >>>>>> become closer to
>> > >>>> >>>>>> > what real
>> > >>>> >>>>>> > >>>>>>>>>
>> > users
>> > >>>> would
>> > >>>> >>>>>> have in their
>> > >>>> >>>>>> > code and is
>> > >>>> >>>>>> > >>>>>>>>>
>> > the
>> > >>>> >>>>>> "recommended" way of doing it
>> > >>>> >>>>>> > >>>>>>>>>
>> > >>>> >>>>>> > >>>>>>>>>
>> > - The
>> > >>>> >>>>>> alternative is to
>> > >>>> >>>>>> > use the
>> > >>>> >>>>>> > >>>>>>>>>
>> > NonFatalError I
>> > >>>> added
>> > >>>> >>>>>> which
>> > >>>> >>>>>> > then just
>> > >>>> >>>>>> > >>>>>>>>>
>> > prints
>> > >>>> out
>> > >>>> >>>>>> that something
>> > >>>> >>>>>> > went wrong,
>> > >>>> >>>>>> > >>>>>>>>>
>> > letting
>> > >>>> the
>> > >>>> >>>>>> test continue.
>> > >>>> >>>>>> > Question
>> > >>>> >>>>>> > >>>>>>>>>
>> > will be
>> > >>>> what
>> > >>>> >>>>>> should be the
>> > >>>> >>>>>> > default?
>> > >>>> >>>>>> > >>>>>>>>>
>> > The
>> > >>>> fatal or
>> > >>>> >>>>>> the non-fatal error
>> > >>>> >>>>>> > >>>>>>>>>
>> > handling?
>> > >>>> >>>>>> > >>>>>>>>>
>> > >>>> >>>>>> > >>>>>>>>>
>> > On a
>> > >>>> >>>>>> different subject:
>> > >>>> >>>>>> > >>>>>>>>>
>> > - On
>> > >>>> the
>> > >>>> >>>>>> new tests, I've
>> > >>>> >>>>>> > removed
>> > >>>> >>>>>> > >>>>>>>>>
>> > the
>> > >>>> >>>>>> NSK_JNI_VERIFY since the JNI
>> > >>>> >>>>>> > >>>>>>>>>
>> > wrapper
>> > >>>> >>>>>> handles the tracing
>> > >>>> >>>>>> > and the
>> > >>>> >>>>>> > >>>>>>>>>
>> > verify in
>> > >>>> >>>>>> almost the same
>> > >>>> >>>>>> > way; only
>> > >>>> >>>>>> > >>>>>>>>>
>> > >>>> difference I
>> > >>>> >>>>>> can really tell
>> > >>>> >>>>>> > is that
>> > >>>> >>>>>> > >>>>>>>>>
>> > the
>> > >>>> complain
>> > >>>> >>>>>> method from NSK
>> > >>>> >>>>>> > has a
>> > >>>> >>>>>> > >>>>>>>>>
>> > max
>> > >>>> complain
>> > >>>> >>>>>> before stopping to
>> > >>>> >>>>>> > >>>>>>>>>
>> > >>>> "complain";
>> > >>>> >>>>>> I have not added that
>> > >>>> >>>>>> > >>>>>>>>>
>> > part of
>> > >>>> the
>> > >>>> >>>>>> code in this webrev
>> > >>>> >>>>>> > >>>>>>>>>
>> > >>>> >>>>>> > >>>>>>>>>
>> > Once we
>> > >>>> >>>>>> decide on these, I can
>> > >>>> >>>>>> > >>>>>>>>>
>> > continue
>> > >>>> on
>> > >>>> >>>>>> the files from
>> > >>>> >>>>>> > >>>>>>>>>
>> > >>>> JDK-8212884
>> > >>>> >>>>>> and then do both the
>> > >>>> >>>>>> > >>>>>>>>>
>> > >>>> assignment
>> > >>>> >>>>>> in an if extraction
>> > >>>> >>>>>> > >>>>>>>>>
>> > >>>> followed-by
>> > >>>> >>>>>> this type of
>> > >>>> >>>>>> > webrev in an
>> > >>>> >>>>>> > >>>>>>>>>
>> > easier
>> > >>>> >>>>>> fashion. Depending on
>> > >>>> >>>>>> > >>>>>>>>>
>> > decisions
>> > >>>> >>>>>> here, NSK*VERIFY can be
>> > >>>> >>>>>> > >>>>>>>>>
>> > >>>> deprecated
>> > >>>> >>>>>> as well as we go
>> > >>>> >>>>>> > forward.
>> > >>>> >>>>>> > >>>>>>>>>
>> > >>>> >>>>>> > >>>>>>>>>
>> > Thank you
>> > >>>> >>>>>> for the
>> > >>>> >>>>>> > reviews/comments :)
>> > >>>> >>>>>> > >>>>>>>>>
>> > Jc
>> > >>>> >>>>>> > >>>>>>>>>
>> > >>>> >>>>>> > >>>>>>>>>
>> > >>>> >>>>>> > >>>>>>>>>
>> > >>>> >>>>>> > >>>>>>>>>
>> --
>> > >>>> >>>>>> > >>>>>>>>>
>> > Thanks,
>> > >>>> >>>>>> > >>>>>>>>>
>> Jc
>> > >>>> >>>>>> > >>>>>>>>
>> > >>>> >>>>>> > >>>>>>>>
>> > >>>> >>>>>> > >>>>>>>>
>> > >>>> >>>>>> > >>>>>>>>
>> --
>> > >>>> >>>>>> > >>>>>>>>
>> > Thanks,
>> > >>>> >>>>>> > >>>>>>>>
>> Jc
>> > >>>> >>>>>> > >>>>>>>
>> > >>>> >>>>>> > >>>>>>>
>> > >>>> >>>>>> > >>>>>>>
>> > >>>> >>>>>> > >>>>>>>
>> > >>>> >>>>>> > >>>>>>> --
>> > >>>> >>>>>> > >>>>>>>
>> Thanks,
>> > >>>> >>>>>> > >>>>>>> Jc
>> > >>>> >>>>>> > >>>>>>
>> > >>>> >>>>>> > >>>>>>
>> > >>>> >>>>>> > >>>>>>
>> > >>>> >>>>>> > >>>>>>
>> > >>>> >>>>>> > >>>>>> --
>> > >>>> >>>>>> > >>>>>> Thanks,
>> > >>>> >>>>>> > >>>>>> Jc
>> > >>>> >>>>>> > >>>>>
>> > >>>> >>>>>> > >>>>>
>> > >>>> >>>>>> > >>>>>
>> > >>>> >>>>>> > >>>>>
>> > >>>> >>>>>> > >>>>> --
>> > >>>> >>>>>> > >>>>> Thanks,
>> > >>>> >>>>>> > >>>>> Jc
>> > >>>> >>>>>> > >>>>
>> > >>>> >>>>>> > >>>
>> > >>>> >>>>>> > >>>
>> > >>>> >>>>>> > >>>
>> > >>>> >>>>>> > >>> --
>> > >>>> >>>>>> > >>> Thanks,
>> > >>>> >>>>>> > >>> Jc
>> > >>>> >>>>>> > >>>
>> > >>>> >>>>>> > >>>
>> > >>>> >>>>>> > >>>
>> > >>>> >>>>>> > >>> --
>> > >>>> >>>>>> > >>>
>> > >>>> >>>>>> > >>> Thanks,
>> > >>>> >>>>>> > >>> Jc
>> > >>>> >>>>>> > >
>> > >>>> >>>>>> > >
>> > >>>> >>>>>> >
>> > >>>> >>>>>> >
>> > >>>> >>>>>> >
>> > >>>> >>>>>> > --
>> > >>>> >>>>>> >
>> > >>>> >>>>>> > Thanks,
>> > >>>> >>>>>> > Jc
>> > >>>> >>>>>>
>> > >>>> >>>>>>
>> > >>>> >>>>>>
>> > >>>> >>>>>> --
>> > >>>> >>>>>> Thanks,
>> > >>>> >>>>>> Jc
>> > >>>> >>>>>
>> > >>>> >>>>>
>> > >>>> >>>>>
>> > >>>> >>>>> --
>> > >>>> >>>>> Thanks,
>> > >>>> >>>>> Jc
>> > >>>> >>>>
>> > >>>> >>>>
>> > >>>> >>>>
>> > >>>> >>>> --
>> > >>>> >>>> Thanks,
>> > >>>> >>>> Jc
>> > >>>> >>>>
>> > >>>> >>>>
>> > >>>> >>>>
>> > >>>> >>>> --
>> > >>>> >>>>
>> > >>>> >>>> Thanks,
>> > >>>> >>>> Jc
>> > >>>> >>>
>> > >>>> >
>> > >>>>
>> > >>>
>> > >>>
>> > >>> --
>> > >>>
>> > >>> Thanks,
>> > >>> Jc
>> > >>>
>> > >>
>> > >>
>> > >> --
>> > >>
>> > >> Thanks,
>> > >> Jc
>> > >>
>> > >
>> > >
>> > > --
>> > >
>> > > Thanks,
>> > > Jc
>> > >
>> >
>> >
>> > --
>> >
>> > Thanks,
>> > Jc
>> >
>> >
>> >
>> >
>> > --
>> >
>> > Thanks,
>> > Jc
>>
>
>
> --
>
> Thanks,
> Jc
>
--
Thanks,
Jc
More information about the hotspot-dev
mailing list