RFR (M) 8212771: Remove remaining spaces before/after () for vmTestbase

Discussion:

JC Beyler

2018-10-22 18:30:49 UTC

Hi all,

Here is the last webrev (3 out of 3) to remove the spaces after/before ()
from vmTestbase. It is straightforward and I generated the webrev with
white-space changes showing up of course:

Webrev: http://cr.openjdk.java.net/~jcbeyler/8212771/webrev.00/
Bug: https://bugs.openjdk.java.net/browse/JDK-8212771

Could I please get some reviews?

Thanks,
Jc

s***@oracle.com

2018-10-22 21:57:12 UTC

Permalink

<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">Hi Jc,<br>
<br>
It looks good.<br>
<br>
Thanks!<br>
Serguei<br>
<br>
<br>
On 10/22/18 11:30, JC Beyler wrote:<br>
</div>
<blockquote type="cite"
cite="mid:CAF9BGBzJ_8znPej_b1Z=***@mail.gmail.com">
<div dir="ltr">
<div dir="ltr">Hi all,</div>
<div dir="ltr"><br>
</div>
<div dir="ltr">Here is the last webrev (3 out of 3) to remove
the spaces after/before () from vmTestbase. It is
straightforward and I generated the webrev with white-space
changes showing up of course:</div>
<div dir="ltr"><br>
</div>
<div dir="ltr">
<div>Webrev: <a
href="http://cr.openjdk.java.net/%7Ejcbeyler/8212771/webrev.00/"
moz-do-not-send="true">http://cr.openjdk.java.net/~jcbeyler/8212771/webrev.00/</a><br>
</div>
<div>Bug: <a
href="https://bugs.openjdk.java.net/browse/JDK-8212771"
moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8212771</a></div>
<div><br>
</div>
<div>Could I please get some reviews?</div>
<div dir="ltr" class="gmail_signature">
<div dir="ltr">
<div><br>
</div>
Thanks,
<div>Jc</div>
</div>
</div>
</div>
</div>
</blockquote>
<br>
</body>
</html>

Hohensee, Paul

2018-10-23 00:10:09 UTC

Permalink

Lgtm.
Thereâs a similar issue with â{ââ}â you might want to fix separately, except in the opposite direction :), e.g., in test/hotspot/jtreg/vmTestbase/nsk/jvmti/unit/MethodBind/JvmtiTest/JvmtiTest.cpp,
+#define JVMTI_ERROR_CHECK(str,res) if (res != JVMTI_ERROR_NONE) { printf(str); printf("%d\n",res); return res;}
+#define JVMTI_ERROR_CHECK_EXPECTED_ERROR(str,res,err) if (res != err) { printf(str); printf("unexpected error %d\n",res); return res;}
=>

+#define JVMTI_ERROR_CHECK(str,res) if (res != JVMTI_ERROR_NONE) { printf(str); printf("%d\n",res); return res; }

+#define JVMTI_ERROR_CHECK_EXPECTED_ERROR(str,res,err) if (res != err) { printf(str); printf("unexpected error %d\n",res); return res; }

And somewhat different in test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/aod/jvmti_aod.cpp

+ (threadConstructor = NSK_CPP_STUB4(GetMethodID, jni, klass, "<init>", "()V")) != NULL)) {

=>

+ (threadConstructor = NSK_CPP_STUB4(GetMethodID, jni, klass, "<init>", "()V")) != NULL)) {

Paul
From: serviceability-dev <serviceability-dev-***@openjdk.java.net> on behalf of "***@oracle.com" <***@oracle.com>
Date: Monday, October 22, 2018 at 3:15 PM
To: JC Beyler <***@google.com>, "serviceability-***@openjdk.java.net" <serviceability-***@openjdk.java.net>
Subject: Re: RFR (M) 8212771: Remove remaining spaces before/after () for vmTestbase

Hi Jc,

It looks good.

Thanks!
Serguei

On 10/22/18 11:30, JC Beyler wrote:
Hi all,

Here is the last webrev (3 out of 3) to remove the spaces after/before () from vmTestbase. It is straightforward and I generated the webrev with white-space changes showing up of course:

Webrev: http://cr.openjdk.java.net/~jcbeyler/8212771/webrev.00/<http://cr.openjdk.java.net/%7Ejcbeyler/8212771/webrev.00/>
Bug: https://bugs.openjdk.java.net/browse/JDK-8212771

Could I please get some reviews?

Thanks,
Jc

JC Beyler

2018-10-23 14:45:36 UTC

Permalink

Hi Paul,

Thanks for the review! I created two new bugs for these two refactoring
cases. They seem fairly uncommon so should be easy to fix throughout .cpp
files in vmTestbase.
https://bugs.openjdk.java.net/browse/JDK-8212822
https://bugs.openjdk.java.net/browse/JDK-8212824

So I'll keep them out of scope of this change as you recommended to do it
separately ;-),
Jc

Post by Hohensee, Paul
Lgtm.
Thereâs a similar issue with â{ââ}â you might want to fix separately,
except in the opposite direction :), e.g., in
test/hotspot/jtreg/vmTestbase/nsk/jvmti/unit/MethodBind/JvmtiTest/JvmtiTest.cpp,
+#define JVMTI_ERROR_CHECK(str,res) if (res != JVMTI_ERROR_NONE) {
printf(str); printf("%d\n",res); return res;}
+#define JVMTI_ERROR_CHECK_EXPECTED_ERROR(str,res,err) if (res != err) {
printf(str); printf("unexpected error %d\n",res); return res;}
=>
+#define JVMTI_ERROR_CHECK(str,res) if (res != JVMTI_ERROR_NONE) { printf(str); printf("%d\n",res); return res; }
+#define JVMTI_ERROR_CHECK_EXPECTED_ERROR(str,res,err) if (res != err) { printf(str); printf("unexpected error %d\n",res); return res; }
And somewhat different in
test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/aod/jvmti_aod.cpp
+ (threadConstructor = NSK_CPP_STUB4(GetMethodID, jni, klass, "<init>", "()V")) != NULL)) {
=>
+ (threadConstructor = NSK_CPP_STUB4(GetMethodID, jni, klass, "<init>", "()V")) != NULL)) {
Paul
*Date: *Monday, October 22, 2018 at 3:15 PM
*Subject: *Re: RFR (M) 8212771: Remove remaining spaces before/after ()
for vmTestbase
Hi Jc,
It looks good.
Thanks!
Serguei
Hi all,
Here is the last webrev (3 out of 3) to remove the spaces after/before ()
from vmTestbase. It is straightforward and I generated the webrev with
Webrev: http://cr.openjdk.java.net/~jcbeyler/8212771/webrev.00/
Bug: https://bugs.openjdk.java.net/browse/JDK-8212771
Could I please get some reviews?
Thanks,
Jc

--
Thanks,
Jc

Hohensee, Paul

2018-10-23 14:55:06 UTC

Permalink

Thanks for filing the new RFEs!

Paul

From: JC Beyler <***@google.com>
Date: Tuesday, October 23, 2018 at 7:49 AM
To: "Hohensee, Paul" <***@amazon.com>
Cc: "***@oracle.com" <***@oracle.com>, "serviceability-***@openjdk.java.net" <serviceability-***@openjdk.java.net>
Subject: Re: RFR (M) 8212771: Remove remaining spaces before/after () for vmTestbase

Hi Paul,

Thanks for the review! I created two new bugs for these two refactoring cases. They seem fairly uncommon so should be easy to fix throughout .cpp files in vmTestbase.
https://bugs.openjdk.java.net/browse/JDK-8212822
https://bugs.openjdk.java.net/browse/JDK-8212824

So I'll keep them out of scope of this change as you recommended to do it separately ;-),
Jc

On Mon, Oct 22, 2018 at 5:10 PM Hohensee, Paul <***@amazon.com<mailto:***@amazon.com>> wrote:
Lgtm.
Thereâs a similar issue with â{ââ}â you might want to fix separately, except in the opposite direction :), e.g., in test/hotspot/jtreg/vmTestbase/nsk/jvmti/unit/MethodBind/JvmtiTest/JvmtiTest.cpp,
+#define JVMTI_ERROR_CHECK(str,res) if (res != JVMTI_ERROR_NONE) { printf(str); printf("%d\n",res); return res;}
+#define JVMTI_ERROR_CHECK_EXPECTED_ERROR(str,res,err) if (res != err) { printf(str); printf("unexpected error %d\n",res); return res;}
=>

+#define JVMTI_ERROR_CHECK(str,res) if (res != JVMTI_ERROR_NONE) { printf(str); printf("%d\n",res); return res; }

+#define JVMTI_ERROR_CHECK_EXPECTED_ERROR(str,res,err) if (res != err) { printf(str); printf("unexpected error %d\n",res); return res; }

And somewhat different in test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/aod/jvmti_aod.cpp

+ (threadConstructor = NSK_CPP_STUB4(GetMethodID, jni, klass, "<init>", "()V")) != NULL)) {

=>

+ (threadConstructor = NSK_CPP_STUB4(GetMethodID, jni, klass, "<init>", "()V")) != NULL)) {

Paul
From: serviceability-dev <serviceability-dev-***@openjdk.java.net<mailto:serviceability-dev-***@openjdk.java.net>> on behalf of "***@oracle.com<mailto:***@oracle.com>" <***@oracle.com<mailto:***@oracle.com>>
Date: Monday, October 22, 2018 at 3:15 PM
To: JC Beyler <***@google.com<mailto:***@google.com>>, "serviceability-***@openjdk.java.net<mailto:serviceability-***@openjdk.java.net>" <serviceability-***@openjdk.java.net<mailto:serviceability-***@openjdk.java.net>>
Subject: Re: RFR (M) 8212771: Remove remaining spaces before/after () for vmTestbase

Hi Jc,

It looks good.

Thanks!
Serguei

On 10/22/18 11:30, JC Beyler wrote:
Hi all,

Here is the last webrev (3 out of 3) to remove the spaces after/before () from vmTestbase. It is straightforward and I generated the webrev with white-space changes showing up of course:

Webrev: http://cr.openjdk.java.net/~jcbeyler/8212771/webrev.00/<http://cr.openjdk.java.net/%7Ejcbeyler/8212771/webrev.00/>
Bug: https://bugs.openjdk.java.net/browse/JDK-8212771

Could I please get some reviews?

Thanks,
Jc

--
Thanks,
Jc