RFR (L) 8213501 : Deploy ExceptionJniWrapper for a few tests
David Holmes
david.holmes at oracle.com
Wed Dec 12 21:52:01 UTC 2018
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 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>> 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> <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>
>>> <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>> 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>>
>>>> <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>>>> 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>>>>
>>>> > 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>>> 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>>>>
>>>> > >>>>> 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>>>> 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>>>> 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>>>
>>>> > >>>>>>>>
>>>> <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>>>> 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
>
More information about the hotspot-dev
mailing list