Re: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument
On 12/6/19 3:04 AM, Langer, Christoph wrote: Thanks, David. I'll run the final change once again through jdk-submit befor pushing. Alan, Dan, may I consider this reviewed by either of you? Sorry I haven't done a full review on this change. I just happened to see the typo fly by... Dan Thanks Christoph -Original Message- From: David Holmes Sent: Freitag, 6. Dezember 2019 08:02 To: Langer, Christoph ; daniel.daughe...@oracle.com Cc: core-libs-dev@openjdk.java.net; hotspot-runtime- d...@openjdk.java.net; Alan Bateman ; gerard ziemski Subject: Re: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument Hi Christoph, On 6/12/2019 2:12 am, Langer, Christoph wrote: Hi David, I prepared an updated webrev: http://cr.openjdk.java.net/~clanger/webrevs/8234185.3/ src/java.base/windows/native/libjava/canonicalize_md.c +// We can't include jdk_util.h here because the file is used in Oracle in another context +// #include "jdk_util.h" Seems to me clients of JDK_Canonicalize need to include jdk_util.h, not the files that implement it. So there is no reason to include it here and so no need for the comment. Well, it's actually not needed but I think it's good practice that the declaring header is included in the implementation file. Yes I was forgetting the importance of ensuring the declaration in the header matches the definition. There is a typo in the comment "possibile". Further, does: src/java.base/unix/native/libjava/canonicalize_md.c need to include jdk_util.h? I think not. Same as for the windows file - not necessary but good style. +/* The appropriate location of getPrefixed() is io_util_md.c, but it is + also used in a non-OpenJDK context within Oracle. There, canonicalize_md.c + is already pulled in and compiled, so to avoid more complicate solutions + we keep this method here. */ I don't like to have comments like this but I guess it is needed here. Please put the */ on a newline. Also s/complicate/complicates/ I did as Dan pointed out. :) Yes sorry about that slip. Otherwise all looks good. No need for new webrev for the typo. Thanks, David src/java.base/windows/native/libjava/io_util_md.c should now be unchanged, but you've left in the copyright update. Right, thanks for the catch. A second review is still needed for the final version of this. Dan, can I add you as reviewer then? Best regards Christoph
RE: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument
Thanks, Alan. Then I'll push it now. A final jdk-submit run succeeded (mach5-one-clanger-JDK-8234185-4-20191206-0819-7283662). > -Original Message- > From: Alan Bateman > Sent: Freitag, 6. Dezember 2019 13:17 > To: Langer, Christoph ; David Holmes > ; daniel.daughe...@oracle.com > Cc: core-libs-dev@openjdk.java.net; hotspot-runtime- > d...@openjdk.java.net; gerard ziemski > Subject: Re: RFR: 8234185: Cleanup usage of canonicalize function between > libjava, hotspot and libinstrument > > On 06/12/2019 08:04, Langer, Christoph wrote: > > Thanks, David. > > > > I'll run the final change once again through jdk-submit befor pushing. > > > > Alan, Dan, may I consider this reviewed by either of you? > > > Yes, I think this looks okay. > > -Alan
Re: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument
On 06/12/2019 08:04, Langer, Christoph wrote: Thanks, David. I'll run the final change once again through jdk-submit befor pushing. Alan, Dan, may I consider this reviewed by either of you? Yes, I think this looks okay. -Alan
RE: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument
Thanks, David. I'll run the final change once again through jdk-submit befor pushing. Alan, Dan, may I consider this reviewed by either of you? Thanks Christoph > -Original Message- > From: David Holmes > Sent: Freitag, 6. Dezember 2019 08:02 > To: Langer, Christoph ; > daniel.daughe...@oracle.com > Cc: core-libs-dev@openjdk.java.net; hotspot-runtime- > d...@openjdk.java.net; Alan Bateman ; gerard > ziemski > Subject: Re: RFR: 8234185: Cleanup usage of canonicalize function between > libjava, hotspot and libinstrument > > Hi Christoph, > > On 6/12/2019 2:12 am, Langer, Christoph wrote: > > Hi David, > > > > I prepared an updated webrev: > http://cr.openjdk.java.net/~clanger/webrevs/8234185.3/ > > > >> src/java.base/windows/native/libjava/canonicalize_md.c > >> > >> +// We can't include jdk_util.h here because the file is used in Oracle > >> in another context > >> +// #include "jdk_util.h" > >> > >> Seems to me clients of JDK_Canonicalize need to include jdk_util.h, not > >> the files that implement it. So there is no reason to include it here > >> and so no need for the comment. > > > > Well, it's actually not needed but I think it's good practice that the > > declaring > header is included in the implementation file. > > Yes I was forgetting the importance of ensuring the declaration in the > header matches the definition. There is a typo in the comment "possibile". > > >> Further, does: > >> > >> src/java.base/unix/native/libjava/canonicalize_md.c > >> > >> need to include jdk_util.h? I think not. > > > > Same as for the windows file - not necessary but good style. > > > >> +/* The appropriate location of getPrefixed() is io_util_md.c, but it is > >> + also used in a non-OpenJDK context within Oracle. There, > >> canonicalize_md.c > >> + is already pulled in and compiled, so to avoid more complicate > >> solutions > >> + we keep this method here. */ > >> > >> I don't like to have comments like this but I guess it is needed here. > >> Please put the */ on a newline. Also s/complicate/complicates/ > > > > I did as Dan pointed out. > > :) Yes sorry about that slip. > > Otherwise all looks good. No need for new webrev for the typo. > > Thanks, > David > > >> src/java.base/windows/native/libjava/io_util_md.c > >> > >> should now be unchanged, but you've left in the copyright update. > >> > > > > Right, thanks for the catch. > > > >> A second review is still needed for the final version of this. > > > > Dan, can I add you as reviewer then? > > > > Best regards > > Christoph > >
Re: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument
Hi Christoph, On 6/12/2019 2:12 am, Langer, Christoph wrote: Hi David, I prepared an updated webrev: http://cr.openjdk.java.net/~clanger/webrevs/8234185.3/ src/java.base/windows/native/libjava/canonicalize_md.c +// We can't include jdk_util.h here because the file is used in Oracle in another context +// #include "jdk_util.h" Seems to me clients of JDK_Canonicalize need to include jdk_util.h, not the files that implement it. So there is no reason to include it here and so no need for the comment. Well, it's actually not needed but I think it's good practice that the declaring header is included in the implementation file. Yes I was forgetting the importance of ensuring the declaration in the header matches the definition. There is a typo in the comment "possibile". Further, does: src/java.base/unix/native/libjava/canonicalize_md.c need to include jdk_util.h? I think not. Same as for the windows file - not necessary but good style. +/* The appropriate location of getPrefixed() is io_util_md.c, but it is + also used in a non-OpenJDK context within Oracle. There, canonicalize_md.c + is already pulled in and compiled, so to avoid more complicate solutions + we keep this method here. */ I don't like to have comments like this but I guess it is needed here. Please put the */ on a newline. Also s/complicate/complicates/ I did as Dan pointed out. :) Yes sorry about that slip. Otherwise all looks good. No need for new webrev for the typo. Thanks, David src/java.base/windows/native/libjava/io_util_md.c should now be unchanged, but you've left in the copyright update. Right, thanks for the catch. A second review is still needed for the final version of this. Dan, can I add you as reviewer then? Best regards Christoph
Re: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument
On 6/12/2019 2:06 am, Daniel D. Daugherty wrote:
On 12/5/19 8:00 AM, David Holmes wrote:
Hi Christoph,
On 5/12/2019 9:55 pm, Langer, Christoph wrote:
Hi David,
thanks again for your efforts.
Here is a version that ran successfully through jdk-submit
(mach5-one-clanger-JDK-8234185-3-20191205-1051-7247172):
http://cr.openjdk.java.net/~clanger/webrevs/8234185.2/
I avoid the inclusion of jdk_util.h in
src/java.base/windows/native/libjava/canonicalize_md.c. Do you think
this is good to go?
Avoiding the include does seem to be the way to go. That seems so
obvious now.
src/java.base/windows/native/libjava/canonicalize_md.c
+// We can't include jdk_util.h here because the file is used in
Oracle in another context
+// #include "jdk_util.h"
Seems to me clients of JDK_Canonicalize need to include jdk_util.h,
not the files that implement it. So there is no reason to include it
here and so no need for the comment. Further, does:
src/java.base/unix/native/libjava/canonicalize_md.c
need to include jdk_util.h? I think not.
+/* The appropriate location of getPrefixed() is io_util_md.c, but it is
+ also used in a non-OpenJDK context within Oracle. There,
canonicalize_md.c
+ is already pulled in and compiled, so to avoid more complicate
solutions
+ we keep this method here. */
I don't like to have comments like this but I guess it is needed here.
Please put the */ on a newline. Also s/complicate/complicates/
It should be:
s/complicate/complicated/
Thanks Dan - fat fingers :) I need a keyboard with bigger spaces between
keys
David
Dan
src/java.base/windows/native/libjava/io_util_md.c
should now be unchanged, but you've left in the copyright update.
A second review is still needed for the final version of this.
Thanks,
David
Somebody in Oracle could then eventually clean up things by
decoupling the installer from OpenJDK's canonicalize_md.c. I'd open a
bug for this.
Thanks
Christoph
-Original Message-
From: David Holmes
Sent: Dienstag, 3. Dezember 2019 23:52
To: Langer, Christoph
Cc: core-libs-dev@openjdk.java.net; hotspot-runtime-
d...@openjdk.java.net; Alan Bateman ; gerard
ziemski
Subject: Re: RFR: 8234185: Cleanup usage of canonicalize function
between
libjava, hotspot and libinstrument
Hi Christoph,
On 3/12/2019 7:26 pm, Langer, Christoph wrote:
Hi David,
thanks for taking care of this.
This would be my updated patch that could hopefully be enabled by just
adding the include directory where "jdk_util.h" is located. It would
be really
great if that'd work. I think it would open up a path to
automatically include
io_util_md.h by including io_util.h.
http://cr.openjdk.java.net/~clanger/webrevs/8234185.1/
I'm afraid I cannot get this to work at our end. I get the following
errors:
t:\workspace\open\src\java.base\share\native\libjava\jdk_util.h(46):
error C2143: syntax error: missing ')' before '*'
t:\workspace\open\src\java.base\share\native\libjava\jdk_util.h(46):
error C2143: syntax error: missing '{' before '*'
t:\workspace\open\src\java.base\share\native\libjava\jdk_util.h(46):
warning C4142: 'size_t': benign redefinition of type
C:\ade\mesos\work_dir\jib-
ma~1\install\jpg\infra\buildd~1\devkit~1\vs2017~3.0\devkit~1.gz\VC\includ
e\vcruntime.h(184):
note: see declaration of 'size_t'
t:\workspace\open\src\java.base\share\native\libjava\jdk_util.h(46):
error C2370: 'size_t': redefinition; different storage class
C:\ade\mesos\work_dir\jib-
ma~1\install\jpg\infra\buildd~1\devkit~1\vs2017~3.0\devkit~1.gz\VC\includ
e\vcruntime.h(184):
note: see declaration of 'size_t'
t:\workspace\open\src\java.base\share\native\libjava\jdk_util.h(46):
error C2146: syntax error: missing ';' before identifier 'info_size'
t:\workspace\open\src\java.base\share\native\libjava\jdk_util.h(46):
error C2059: syntax error: ')'
This pertains to the line:
JDK_GetVersionInfo0(jdk_version_info* info, size_t info_size);
There is also a second problem in our closed code that will take more
effort from someone familiar with that code to resolve. I will file an
issue for our install team to pick up.
I would ask that this not be pushed for the moment while others figure
out how best to handle this.
Thanks,
David
-
Cheers
Christoph
-Original Message-
From: David Holmes
Sent: Dienstag, 3. Dezember 2019 03:13
To: Langer, Christoph ; Alan Bateman
; gerard ziemski
Cc: core-libs-dev@openjdk.java.net; hotspot-runtime-
d...@openjdk.java.net
Subject: Re: RFR: 8234185: Cleanup usage of canonicalize function
between
libjava, hotspot and libinstrument
Hi Christoph,
Can you please post your updated patch for review and I will use
it to
check the fix for our internal build. Once you have your fix
reviewed I
will push both fixes together.
Thanks,
David
On 25/11/2019 10:4
RE: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument
Hi David, I prepared an updated webrev: http://cr.openjdk.java.net/~clanger/webrevs/8234185.3/ > src/java.base/windows/native/libjava/canonicalize_md.c > > +// We can't include jdk_util.h here because the file is used in Oracle > in another context > +// #include "jdk_util.h" > > Seems to me clients of JDK_Canonicalize need to include jdk_util.h, not > the files that implement it. So there is no reason to include it here > and so no need for the comment. Well, it's actually not needed but I think it's good practice that the declaring header is included in the implementation file. > Further, does: > > src/java.base/unix/native/libjava/canonicalize_md.c > > need to include jdk_util.h? I think not. Same as for the windows file - not necessary but good style. > +/* The appropriate location of getPrefixed() is io_util_md.c, but it is > + also used in a non-OpenJDK context within Oracle. There, > canonicalize_md.c > + is already pulled in and compiled, so to avoid more complicate solutions > + we keep this method here. */ > > I don't like to have comments like this but I guess it is needed here. > Please put the */ on a newline. Also s/complicate/complicates/ I did as Dan pointed out. > src/java.base/windows/native/libjava/io_util_md.c > > should now be unchanged, but you've left in the copyright update. > Right, thanks for the catch. > A second review is still needed for the final version of this. Dan, can I add you as reviewer then? Best regards Christoph
Re: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument
On 12/5/19 8:00 AM, David Holmes wrote:
Hi Christoph,
On 5/12/2019 9:55 pm, Langer, Christoph wrote:
Hi David,
thanks again for your efforts.
Here is a version that ran successfully through jdk-submit
(mach5-one-clanger-JDK-8234185-3-20191205-1051-7247172):
http://cr.openjdk.java.net/~clanger/webrevs/8234185.2/
I avoid the inclusion of jdk_util.h in
src/java.base/windows/native/libjava/canonicalize_md.c. Do you think
this is good to go?
Avoiding the include does seem to be the way to go. That seems so
obvious now.
src/java.base/windows/native/libjava/canonicalize_md.c
+// We can't include jdk_util.h here because the file is used in
Oracle in another context
+// #include "jdk_util.h"
Seems to me clients of JDK_Canonicalize need to include jdk_util.h,
not the files that implement it. So there is no reason to include it
here and so no need for the comment. Further, does:
src/java.base/unix/native/libjava/canonicalize_md.c
need to include jdk_util.h? I think not.
+/* The appropriate location of getPrefixed() is io_util_md.c, but it is
+ also used in a non-OpenJDK context within Oracle. There,
canonicalize_md.c
+ is already pulled in and compiled, so to avoid more complicate
solutions
+ we keep this method here. */
I don't like to have comments like this but I guess it is needed here.
Please put the */ on a newline. Also s/complicate/complicates/
It should be:
s/complicate/complicated/
Dan
src/java.base/windows/native/libjava/io_util_md.c
should now be unchanged, but you've left in the copyright update.
A second review is still needed for the final version of this.
Thanks,
David
Somebody in Oracle could then eventually clean up things by
decoupling the installer from OpenJDK's canonicalize_md.c. I'd open a
bug for this.
Thanks
Christoph
-Original Message-
From: David Holmes
Sent: Dienstag, 3. Dezember 2019 23:52
To: Langer, Christoph
Cc: core-libs-dev@openjdk.java.net; hotspot-runtime-
d...@openjdk.java.net; Alan Bateman ; gerard
ziemski
Subject: Re: RFR: 8234185: Cleanup usage of canonicalize function
between
libjava, hotspot and libinstrument
Hi Christoph,
On 3/12/2019 7:26 pm, Langer, Christoph wrote:
Hi David,
thanks for taking care of this.
This would be my updated patch that could hopefully be enabled by just
adding the include directory where "jdk_util.h" is located. It would
be really
great if that'd work. I think it would open up a path to
automatically include
io_util_md.h by including io_util.h.
http://cr.openjdk.java.net/~clanger/webrevs/8234185.1/
I'm afraid I cannot get this to work at our end. I get the following
errors:
t:\workspace\open\src\java.base\share\native\libjava\jdk_util.h(46):
error C2143: syntax error: missing ')' before '*'
t:\workspace\open\src\java.base\share\native\libjava\jdk_util.h(46):
error C2143: syntax error: missing '{' before '*'
t:\workspace\open\src\java.base\share\native\libjava\jdk_util.h(46):
warning C4142: 'size_t': benign redefinition of type
C:\ade\mesos\work_dir\jib-
ma~1\install\jpg\infra\buildd~1\devkit~1\vs2017~3.0\devkit~1.gz\VC\includ
e\vcruntime.h(184):
note: see declaration of 'size_t'
t:\workspace\open\src\java.base\share\native\libjava\jdk_util.h(46):
error C2370: 'size_t': redefinition; different storage class
C:\ade\mesos\work_dir\jib-
ma~1\install\jpg\infra\buildd~1\devkit~1\vs2017~3.0\devkit~1.gz\VC\includ
e\vcruntime.h(184):
note: see declaration of 'size_t'
t:\workspace\open\src\java.base\share\native\libjava\jdk_util.h(46):
error C2146: syntax error: missing ';' before identifier 'info_size'
t:\workspace\open\src\java.base\share\native\libjava\jdk_util.h(46):
error C2059: syntax error: ')'
This pertains to the line:
JDK_GetVersionInfo0(jdk_version_info* info, size_t info_size);
There is also a second problem in our closed code that will take more
effort from someone familiar with that code to resolve. I will file an
issue for our install team to pick up.
I would ask that this not be pushed for the moment while others figure
out how best to handle this.
Thanks,
David
-
Cheers
Christoph
-Original Message-
From: David Holmes
Sent: Dienstag, 3. Dezember 2019 03:13
To: Langer, Christoph ; Alan Bateman
; gerard ziemski
Cc: core-libs-dev@openjdk.java.net; hotspot-runtime-
d...@openjdk.java.net
Subject: Re: RFR: 8234185: Cleanup usage of canonicalize function
between
libjava, hotspot and libinstrument
Hi Christoph,
Can you please post your updated patch for review and I will use
it to
check the fix for our internal build. Once you have your fix
reviewed I
will push both fixes together.
Thanks,
David
On 25/11/2019 10:41 pm, David Holmes wrote:
Hi Christoph,
On 25/11/2019 10:38 pm, Langer, Christoph wrote:
Hi David,
thanks for your investigation. I'll prepare a
Re: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument
Hi Christoph,
On 5/12/2019 9:55 pm, Langer, Christoph wrote:
Hi David,
thanks again for your efforts.
Here is a version that ran successfully through jdk-submit
(mach5-one-clanger-JDK-8234185-3-20191205-1051-7247172):
http://cr.openjdk.java.net/~clanger/webrevs/8234185.2/
I avoid the inclusion of jdk_util.h in
src/java.base/windows/native/libjava/canonicalize_md.c. Do you think this is
good to go?
Avoiding the include does seem to be the way to go. That seems so
obvious now.
src/java.base/windows/native/libjava/canonicalize_md.c
+// We can't include jdk_util.h here because the file is used in Oracle
in another context
+// #include "jdk_util.h"
Seems to me clients of JDK_Canonicalize need to include jdk_util.h, not
the files that implement it. So there is no reason to include it here
and so no need for the comment. Further, does:
src/java.base/unix/native/libjava/canonicalize_md.c
need to include jdk_util.h? I think not.
+/* The appropriate location of getPrefixed() is io_util_md.c, but it is
+ also used in a non-OpenJDK context within Oracle. There,
canonicalize_md.c
+ is already pulled in and compiled, so to avoid more complicate solutions
+ we keep this method here. */
I don't like to have comments like this but I guess it is needed here.
Please put the */ on a newline. Also s/complicate/complicates/
src/java.base/windows/native/libjava/io_util_md.c
should now be unchanged, but you've left in the copyright update.
A second review is still needed for the final version of this.
Thanks,
David
Somebody in Oracle could then eventually clean up things by decoupling the
installer from OpenJDK's canonicalize_md.c. I'd open a bug for this.
Thanks
Christoph
-Original Message-
From: David Holmes
Sent: Dienstag, 3. Dezember 2019 23:52
To: Langer, Christoph
Cc: core-libs-dev@openjdk.java.net; hotspot-runtime-
d...@openjdk.java.net; Alan Bateman ; gerard
ziemski
Subject: Re: RFR: 8234185: Cleanup usage of canonicalize function between
libjava, hotspot and libinstrument
Hi Christoph,
On 3/12/2019 7:26 pm, Langer, Christoph wrote:
Hi David,
thanks for taking care of this.
This would be my updated patch that could hopefully be enabled by just
adding the include directory where "jdk_util.h" is located. It would be really
great if that'd work. I think it would open up a path to automatically include
io_util_md.h by including io_util.h.
http://cr.openjdk.java.net/~clanger/webrevs/8234185.1/
I'm afraid I cannot get this to work at our end. I get the following errors:
t:\workspace\open\src\java.base\share\native\libjava\jdk_util.h(46):
error C2143: syntax error: missing ')' before '*'
t:\workspace\open\src\java.base\share\native\libjava\jdk_util.h(46):
error C2143: syntax error: missing '{' before '*'
t:\workspace\open\src\java.base\share\native\libjava\jdk_util.h(46):
warning C4142: 'size_t': benign redefinition of type
C:\ade\mesos\work_dir\jib-
ma~1\install\jpg\infra\buildd~1\devkit~1\vs2017~3.0\devkit~1.gz\VC\includ
e\vcruntime.h(184):
note: see declaration of 'size_t'
t:\workspace\open\src\java.base\share\native\libjava\jdk_util.h(46):
error C2370: 'size_t': redefinition; different storage class
C:\ade\mesos\work_dir\jib-
ma~1\install\jpg\infra\buildd~1\devkit~1\vs2017~3.0\devkit~1.gz\VC\includ
e\vcruntime.h(184):
note: see declaration of 'size_t'
t:\workspace\open\src\java.base\share\native\libjava\jdk_util.h(46):
error C2146: syntax error: missing ';' before identifier 'info_size'
t:\workspace\open\src\java.base\share\native\libjava\jdk_util.h(46):
error C2059: syntax error: ')'
This pertains to the line:
JDK_GetVersionInfo0(jdk_version_info* info, size_t info_size);
There is also a second problem in our closed code that will take more
effort from someone familiar with that code to resolve. I will file an
issue for our install team to pick up.
I would ask that this not be pushed for the moment while others figure
out how best to handle this.
Thanks,
David
-
Cheers
Christoph
-Original Message-
From: David Holmes
Sent: Dienstag, 3. Dezember 2019 03:13
To: Langer, Christoph ; Alan Bateman
; gerard ziemski
Cc: core-libs-dev@openjdk.java.net; hotspot-runtime-
d...@openjdk.java.net
Subject: Re: RFR: 8234185: Cleanup usage of canonicalize function
between
libjava, hotspot and libinstrument
Hi Christoph,
Can you please post your updated patch for review and I will use it to
check the fix for our internal build. Once you have your fix reviewed I
will push both fixes together.
Thanks,
David
On 25/11/2019 10:41 pm, David Holmes wrote:
Hi Christoph,
On 25/11/2019 10:38 pm, Langer, Christoph wrote:
Hi David,
thanks for your investigation. I'll prepare a fix to move back
getPrefixed into canonicalize_md.c. However, could you please still
fix your internal bu
RE: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument
Hi David,
thanks again for your efforts.
Here is a version that ran successfully through jdk-submit
(mach5-one-clanger-JDK-8234185-3-20191205-1051-7247172):
http://cr.openjdk.java.net/~clanger/webrevs/8234185.2/
I avoid the inclusion of jdk_util.h in
src/java.base/windows/native/libjava/canonicalize_md.c. Do you think this is
good to go?
Somebody in Oracle could then eventually clean up things by decoupling the
installer from OpenJDK's canonicalize_md.c. I'd open a bug for this.
Thanks
Christoph
> -Original Message-
> From: David Holmes
> Sent: Dienstag, 3. Dezember 2019 23:52
> To: Langer, Christoph
> Cc: core-libs-dev@openjdk.java.net; hotspot-runtime-
> d...@openjdk.java.net; Alan Bateman ; gerard
> ziemski
> Subject: Re: RFR: 8234185: Cleanup usage of canonicalize function between
> libjava, hotspot and libinstrument
>
> Hi Christoph,
>
> On 3/12/2019 7:26 pm, Langer, Christoph wrote:
> > Hi David,
> >
> > thanks for taking care of this.
> >
> > This would be my updated patch that could hopefully be enabled by just
> adding the include directory where "jdk_util.h" is located. It would be really
> great if that'd work. I think it would open up a path to automatically include
> io_util_md.h by including io_util.h.
> >
> > http://cr.openjdk.java.net/~clanger/webrevs/8234185.1/
>
> I'm afraid I cannot get this to work at our end. I get the following errors:
>
> t:\workspace\open\src\java.base\share\native\libjava\jdk_util.h(46):
> error C2143: syntax error: missing ')' before '*'
> t:\workspace\open\src\java.base\share\native\libjava\jdk_util.h(46):
> error C2143: syntax error: missing '{' before '*'
> t:\workspace\open\src\java.base\share\native\libjava\jdk_util.h(46):
> warning C4142: 'size_t': benign redefinition of type
> C:\ade\mesos\work_dir\jib-
> ma~1\install\jpg\infra\buildd~1\devkit~1\vs2017~3.0\devkit~1.gz\VC\includ
> e\vcruntime.h(184):
> note: see declaration of 'size_t'
> t:\workspace\open\src\java.base\share\native\libjava\jdk_util.h(46):
> error C2370: 'size_t': redefinition; different storage class
> C:\ade\mesos\work_dir\jib-
> ma~1\install\jpg\infra\buildd~1\devkit~1\vs2017~3.0\devkit~1.gz\VC\includ
> e\vcruntime.h(184):
> note: see declaration of 'size_t'
> t:\workspace\open\src\java.base\share\native\libjava\jdk_util.h(46):
> error C2146: syntax error: missing ';' before identifier 'info_size'
> t:\workspace\open\src\java.base\share\native\libjava\jdk_util.h(46):
> error C2059: syntax error: ')'
>
> This pertains to the line:
>
> JDK_GetVersionInfo0(jdk_version_info* info, size_t info_size);
>
> There is also a second problem in our closed code that will take more
> effort from someone familiar with that code to resolve. I will file an
> issue for our install team to pick up.
>
> I would ask that this not be pushed for the moment while others figure
> out how best to handle this.
>
> Thanks,
> David
> -----
>
>
> > Cheers
> > Christoph
> >
> >
> >> -Original Message-
> >> From: David Holmes
> >> Sent: Dienstag, 3. Dezember 2019 03:13
> >> To: Langer, Christoph ; Alan Bateman
> >> ; gerard ziemski
>
> >> Cc: core-libs-dev@openjdk.java.net; hotspot-runtime-
> >> d...@openjdk.java.net
> >> Subject: Re: RFR: 8234185: Cleanup usage of canonicalize function
> between
> >> libjava, hotspot and libinstrument
> >>
> >> Hi Christoph,
> >>
> >> Can you please post your updated patch for review and I will use it to
> >> check the fix for our internal build. Once you have your fix reviewed I
> >> will push both fixes together.
> >>
> >> Thanks,
> >> David
> >>
> >> On 25/11/2019 10:41 pm, David Holmes wrote:
> >>> Hi Christoph,
> >>>
> >>> On 25/11/2019 10:38 pm, Langer, Christoph wrote:
> >>>> Hi David,
> >>>>
> >>>> thanks for your investigation. I'll prepare a fix to move back
> >>>> getPrefixed into canonicalize_md.c. However, could you please still
> >>>> fix your internal build in terms that it would have 'jdk_util.h' in
> >>>> the include path?
> >>>
> >>> That should be simple enough to do.
> >>>
> >>> David
> >>>
> >>>> Thanks
> >>>> Christoph
> >>>>
> >>>>> -Original Message-
> >>>>> From: David Holmes
>
Re: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument
Hi Christoph,
On 3/12/2019 7:26 pm, Langer, Christoph wrote:
Hi David,
thanks for taking care of this.
This would be my updated patch that could hopefully be enabled by just adding the include
directory where "jdk_util.h" is located. It would be really great if that'd
work. I think it would open up a path to automatically include io_util_md.h by including
io_util.h.
http://cr.openjdk.java.net/~clanger/webrevs/8234185.1/
I'm afraid I cannot get this to work at our end. I get the following errors:
t:\workspace\open\src\java.base\share\native\libjava\jdk_util.h(46):
error C2143: syntax error: missing ')' before '*'
t:\workspace\open\src\java.base\share\native\libjava\jdk_util.h(46):
error C2143: syntax error: missing '{' before '*'
t:\workspace\open\src\java.base\share\native\libjava\jdk_util.h(46):
warning C4142: 'size_t': benign redefinition of type
C:\ade\mesos\work_dir\jib-ma~1\install\jpg\infra\buildd~1\devkit~1\vs2017~3.0\devkit~1.gz\VC\include\vcruntime.h(184):
note: see declaration of 'size_t'
t:\workspace\open\src\java.base\share\native\libjava\jdk_util.h(46):
error C2370: 'size_t': redefinition; different storage class
C:\ade\mesos\work_dir\jib-ma~1\install\jpg\infra\buildd~1\devkit~1\vs2017~3.0\devkit~1.gz\VC\include\vcruntime.h(184):
note: see declaration of 'size_t'
t:\workspace\open\src\java.base\share\native\libjava\jdk_util.h(46):
error C2146: syntax error: missing ';' before identifier 'info_size'
t:\workspace\open\src\java.base\share\native\libjava\jdk_util.h(46):
error C2059: syntax error: ')'
This pertains to the line:
JDK_GetVersionInfo0(jdk_version_info* info, size_t info_size);
There is also a second problem in our closed code that will take more
effort from someone familiar with that code to resolve. I will file an
issue for our install team to pick up.
I would ask that this not be pushed for the moment while others figure
out how best to handle this.
Thanks,
David
-
Cheers
Christoph
-Original Message-
From: David Holmes
Sent: Dienstag, 3. Dezember 2019 03:13
To: Langer, Christoph ; Alan Bateman
; gerard ziemski
Cc: core-libs-dev@openjdk.java.net; hotspot-runtime-
d...@openjdk.java.net
Subject: Re: RFR: 8234185: Cleanup usage of canonicalize function between
libjava, hotspot and libinstrument
Hi Christoph,
Can you please post your updated patch for review and I will use it to
check the fix for our internal build. Once you have your fix reviewed I
will push both fixes together.
Thanks,
David
On 25/11/2019 10:41 pm, David Holmes wrote:
Hi Christoph,
On 25/11/2019 10:38 pm, Langer, Christoph wrote:
Hi David,
thanks for your investigation. I'll prepare a fix to move back
getPrefixed into canonicalize_md.c. However, could you please still
fix your internal build in terms that it would have 'jdk_util.h' in
the include path?
That should be simple enough to do.
David
Thanks
Christoph
-Original Message-
From: David Holmes
Sent: Montag, 25. November 2019 01:02
To: Langer, Christoph ; Alan Bateman
; gerard ziemski
Cc: core-libs-dev@openjdk.java.net; hotspot-runtime-
d...@openjdk.java.net
Subject: Re: RFR: 8234185: Cleanup usage of canonicalize function
between
libjava, hotspot and libinstrument
On 25/11/2019 8:45 am, David Holmes wrote:
On 25/11/2019 7:49 am, David Holmes wrote:
On 25/11/2019 7:33 am, David Holmes wrote:
Hi Christoph,
On 23/11/2019 12:04 am, Langer, Christoph wrote:
Hi,
I'd like to push this change. However, running it through jdk-submit
shows reproducible errors:
Job: mach5-one-clanger-JDK-8234185-1-20191122-0927-6913189
BuildId: 2019-11-22-0926373.christoph.langer.source
No failed tests
Tasks Summary
• NA: 0
• NOTHING_TO_RUN: 0
• KILLED: 0
• PASSED: 76
• UNABLE_TO_RUN: 0
• EXECUTED_WITH_FAILURE: 1
• FAILED: 0
• HARNESS_ERROR: 0
Build
1 Executed with failure
o windows-x64-install-windows-x64-build-19 error while building,
return value: 2
Job: mach5-one-clanger-JDK-8234185-20191121-2313-6898791
BuildId: 2019-11-21-2311357.christoph.langer.source
No failed tests
Tasks Summary
• NA: 0
• NOTHING_TO_RUN: 0
• KILLED: 0
• PASSED: 76
• UNABLE_TO_RUN: 0
• EXECUTED_WITH_FAILURE: 1
• FAILED: 0
• HARNESS_ERROR: 0
Build
1 Executed with failure
o windows-x64-install-windows-x64-build-19 error while building,
return value: 2
David already had a look and let me know that the following was
the
reason:
t:/workspace/open/src/java.base/windows/native/libjava/canonicalize_md.
c(41):
fatal error C1083: Cannot open include file: 'jdk_util.h': No such
file or directory
This is not explainable to me as I see this running through my local
build and our nightly builds without problems. I also can't explain
jdk_util.h can't be opened at this place - it should be there and
part of th
RE: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument
Hi David, thanks for taking care of this. This would be my updated patch that could hopefully be enabled by just adding the include directory where "jdk_util.h" is located. It would be really great if that'd work. I think it would open up a path to automatically include io_util_md.h by including io_util.h. http://cr.openjdk.java.net/~clanger/webrevs/8234185.1/ Cheers Christoph > -Original Message- > From: David Holmes > Sent: Dienstag, 3. Dezember 2019 03:13 > To: Langer, Christoph ; Alan Bateman > ; gerard ziemski > Cc: core-libs-dev@openjdk.java.net; hotspot-runtime- > d...@openjdk.java.net > Subject: Re: RFR: 8234185: Cleanup usage of canonicalize function between > libjava, hotspot and libinstrument > > Hi Christoph, > > Can you please post your updated patch for review and I will use it to > check the fix for our internal build. Once you have your fix reviewed I > will push both fixes together. > > Thanks, > David > > On 25/11/2019 10:41 pm, David Holmes wrote: > > Hi Christoph, > > > > On 25/11/2019 10:38 pm, Langer, Christoph wrote: > >> Hi David, > >> > >> thanks for your investigation. I'll prepare a fix to move back > >> getPrefixed into canonicalize_md.c. However, could you please still > >> fix your internal build in terms that it would have 'jdk_util.h' in > >> the include path? > > > > That should be simple enough to do. > > > > David > > > >> Thanks > >> Christoph > >> > >>> -Original Message- > >>> From: David Holmes > >>> Sent: Montag, 25. November 2019 01:02 > >>> To: Langer, Christoph ; Alan Bateman > >>> ; gerard ziemski > > >>> Cc: core-libs-dev@openjdk.java.net; hotspot-runtime- > >>> d...@openjdk.java.net > >>> Subject: Re: RFR: 8234185: Cleanup usage of canonicalize function > >>> between > >>> libjava, hotspot and libinstrument > >>> > >>> > >>> > >>> On 25/11/2019 8:45 am, David Holmes wrote: > >>>> On 25/11/2019 7:49 am, David Holmes wrote: > >>>>> On 25/11/2019 7:33 am, David Holmes wrote: > >>>>>> Hi Christoph, > >>>>>> > >>>>>> On 23/11/2019 12:04 am, Langer, Christoph wrote: > >>>>>>> Hi, > >>>>>>> > >>>>>>> I'd like to push this change. However, running it through jdk-submit > >>>>>>> shows reproducible errors: > >>>>>>> > >>>>>>> Job: mach5-one-clanger-JDK-8234185-1-20191122-0927-6913189 > >>>>>>> BuildId: 2019-11-22-0926373.christoph.langer.source > >>>>>>> No failed tests > >>>>>>> Tasks Summary > >>>>>>> • NA: 0 > >>>>>>> • NOTHING_TO_RUN: 0 > >>>>>>> • KILLED: 0 > >>>>>>> • PASSED: 76 > >>>>>>> • UNABLE_TO_RUN: 0 > >>>>>>> • EXECUTED_WITH_FAILURE: 1 > >>>>>>> • FAILED: 0 > >>>>>>> • HARNESS_ERROR: 0 > >>>>>>> Build > >>>>>>> 1 Executed with failure > >>>>>>> o windows-x64-install-windows-x64-build-19 error while building, > >>>>>>> return value: 2 > >>>>>>> > >>>>>>> > >>>>>>> Job: mach5-one-clanger-JDK-8234185-20191121-2313-6898791 > >>>>>>> BuildId: 2019-11-21-2311357.christoph.langer.source > >>>>>>> No failed tests > >>>>>>> Tasks Summary > >>>>>>> • NA: 0 > >>>>>>> • NOTHING_TO_RUN: 0 > >>>>>>> • KILLED: 0 > >>>>>>> • PASSED: 76 > >>>>>>> • UNABLE_TO_RUN: 0 > >>>>>>> • EXECUTED_WITH_FAILURE: 1 > >>>>>>> • FAILED: 0 > >>>>>>> • HARNESS_ERROR: 0 > >>>>>>> Build > >>>>>>> 1 Executed with failure > >>>>>>> o windows-x64-install-windows-x64-build-19 error while building, > >>>>>>> return value: 2 > >>>>>>> > >>>>>>> > >>>>>>> David already had a look and let me know that the following was > the > >>&g
Re: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument
Hi Christoph, Can you please post your updated patch for review and I will use it to check the fix for our internal build. Once you have your fix reviewed I will push both fixes together. Thanks, David On 25/11/2019 10:41 pm, David Holmes wrote: Hi Christoph, On 25/11/2019 10:38 pm, Langer, Christoph wrote: Hi David, thanks for your investigation. I'll prepare a fix to move back getPrefixed into canonicalize_md.c. However, could you please still fix your internal build in terms that it would have 'jdk_util.h' in the include path? That should be simple enough to do. David Thanks Christoph -Original Message- From: David Holmes Sent: Montag, 25. November 2019 01:02 To: Langer, Christoph ; Alan Bateman ; gerard ziemski Cc: core-libs-dev@openjdk.java.net; hotspot-runtime- d...@openjdk.java.net Subject: Re: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument On 25/11/2019 8:45 am, David Holmes wrote: On 25/11/2019 7:49 am, David Holmes wrote: On 25/11/2019 7:33 am, David Holmes wrote: Hi Christoph, On 23/11/2019 12:04 am, Langer, Christoph wrote: Hi, I'd like to push this change. However, running it through jdk-submit shows reproducible errors: Job: mach5-one-clanger-JDK-8234185-1-20191122-0927-6913189 BuildId: 2019-11-22-0926373.christoph.langer.source No failed tests Tasks Summary • NA: 0 • NOTHING_TO_RUN: 0 • KILLED: 0 • PASSED: 76 • UNABLE_TO_RUN: 0 • EXECUTED_WITH_FAILURE: 1 • FAILED: 0 • HARNESS_ERROR: 0 Build 1 Executed with failure o windows-x64-install-windows-x64-build-19 error while building, return value: 2 Job: mach5-one-clanger-JDK-8234185-20191121-2313-6898791 BuildId: 2019-11-21-2311357.christoph.langer.source No failed tests Tasks Summary • NA: 0 • NOTHING_TO_RUN: 0 • KILLED: 0 • PASSED: 76 • UNABLE_TO_RUN: 0 • EXECUTED_WITH_FAILURE: 1 • FAILED: 0 • HARNESS_ERROR: 0 Build 1 Executed with failure o windows-x64-install-windows-x64-build-19 error while building, return value: 2 David already had a look and let me know that the following was the reason: t:/workspace/open/src/java.base/windows/native/libjava/canonicalize_md. c(41): fatal error C1083: Cannot open include file: 'jdk_util.h': No such file or directory This is not explainable to me as I see this running through my local build and our nightly builds without problems. I also can't explain jdk_util.h can't be opened at this place - it should be there and part of the include directories... I'd appreciate any help... I just dug a little deeper and this is failing in part of our closed build for the install repo. There is a library there that is using canonicalize_md.c directly - i.e. it adds that file to its source files list. The build instructions don't include that directory on the include directory list - hence the failure. But it will also fail due to the name change you made. Actually it appears that the other source code doesn't actually refer to the canonicalize function at all, so a simple fix may be possible at the build level on our side. I'm testing that now. It isn't the canonicalize function that is used, it is getPrefixed, which has now been moved to the io_util_md.c file. So a fix will be a bit more involved. I tried adding io_util_md.c to the library source list instead of canonicalize_md.c but that just caused a slew of other compilation failures, so I don't see any quick fix for us here. David David David - Someone will need to work with you to make the necessary changes to our code. David Thanks Christoph -Original Message- From: Langer, Christoph Sent: Donnerstag, 21. November 2019 14:19 To: Alan Bateman ; core-libs- d...@openjdk.java.net; hotspot-runtime-...@openjdk.java.net Subject: RE: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument Hi Alan, thanks for the review. I'll push it then after running through jdk-submit. /Christoph -Original Message- From: Alan Bateman Sent: Donnerstag, 21. November 2019 09:51 To: Langer, Christoph ; core-libs- d...@openjdk.java.net; hotspot-runtime-...@openjdk.java.net Subject: Re: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument On 14/11/2019 15:37, Langer, Christoph wrote: Hi, please review this cleanup change regarding function "canonicalize" of libjava. Bug: https://bugs.openjdk.java.net/browse/JDK-8234185 Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8234185.0/ The goal is to cleanup how this function is defined and used. One thing is, that there was an unnecessary wrapper function "Canonicalize" in jni_util.c. It wrapped the call to "canonicalize". We can get rid of this wrapper. Unfortunately, it is not possible to just export "canonicalize" since this will
Re: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument
Hi Christoph, On 25/11/2019 10:38 pm, Langer, Christoph wrote: Hi David, thanks for your investigation. I'll prepare a fix to move back getPrefixed into canonicalize_md.c. However, could you please still fix your internal build in terms that it would have 'jdk_util.h' in the include path? That should be simple enough to do. David Thanks Christoph -Original Message- From: David Holmes Sent: Montag, 25. November 2019 01:02 To: Langer, Christoph ; Alan Bateman ; gerard ziemski Cc: core-libs-dev@openjdk.java.net; hotspot-runtime- d...@openjdk.java.net Subject: Re: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument On 25/11/2019 8:45 am, David Holmes wrote: On 25/11/2019 7:49 am, David Holmes wrote: On 25/11/2019 7:33 am, David Holmes wrote: Hi Christoph, On 23/11/2019 12:04 am, Langer, Christoph wrote: Hi, I'd like to push this change. However, running it through jdk-submit shows reproducible errors: Job: mach5-one-clanger-JDK-8234185-1-20191122-0927-6913189 BuildId: 2019-11-22-0926373.christoph.langer.source No failed tests Tasks Summary • NA: 0 • NOTHING_TO_RUN: 0 • KILLED: 0 • PASSED: 76 • UNABLE_TO_RUN: 0 • EXECUTED_WITH_FAILURE: 1 • FAILED: 0 • HARNESS_ERROR: 0 Build 1 Executed with failure o windows-x64-install-windows-x64-build-19 error while building, return value: 2 Job: mach5-one-clanger-JDK-8234185-20191121-2313-6898791 BuildId: 2019-11-21-2311357.christoph.langer.source No failed tests Tasks Summary • NA: 0 • NOTHING_TO_RUN: 0 • KILLED: 0 • PASSED: 76 • UNABLE_TO_RUN: 0 • EXECUTED_WITH_FAILURE: 1 • FAILED: 0 • HARNESS_ERROR: 0 Build 1 Executed with failure o windows-x64-install-windows-x64-build-19 error while building, return value: 2 David already had a look and let me know that the following was the reason: t:/workspace/open/src/java.base/windows/native/libjava/canonicalize_md. c(41): fatal error C1083: Cannot open include file: 'jdk_util.h': No such file or directory This is not explainable to me as I see this running through my local build and our nightly builds without problems. I also can't explain jdk_util.h can't be opened at this place - it should be there and part of the include directories... I'd appreciate any help... I just dug a little deeper and this is failing in part of our closed build for the install repo. There is a library there that is using canonicalize_md.c directly - i.e. it adds that file to its source files list. The build instructions don't include that directory on the include directory list - hence the failure. But it will also fail due to the name change you made. Actually it appears that the other source code doesn't actually refer to the canonicalize function at all, so a simple fix may be possible at the build level on our side. I'm testing that now. It isn't the canonicalize function that is used, it is getPrefixed, which has now been moved to the io_util_md.c file. So a fix will be a bit more involved. I tried adding io_util_md.c to the library source list instead of canonicalize_md.c but that just caused a slew of other compilation failures, so I don't see any quick fix for us here. David David David - Someone will need to work with you to make the necessary changes to our code. David Thanks Christoph -Original Message- From: Langer, Christoph Sent: Donnerstag, 21. November 2019 14:19 To: Alan Bateman ; core-libs- d...@openjdk.java.net; hotspot-runtime-...@openjdk.java.net Subject: RE: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument Hi Alan, thanks for the review. I'll push it then after running through jdk-submit. /Christoph -Original Message- From: Alan Bateman Sent: Donnerstag, 21. November 2019 09:51 To: Langer, Christoph ; core-libs- d...@openjdk.java.net; hotspot-runtime-...@openjdk.java.net Subject: Re: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument On 14/11/2019 15:37, Langer, Christoph wrote: Hi, please review this cleanup change regarding function "canonicalize" of libjava. Bug: https://bugs.openjdk.java.net/browse/JDK-8234185 Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8234185.0/ The goal is to cleanup how this function is defined and used. One thing is, that there was an unnecessary wrapper function "Canonicalize" in jni_util.c. It wrapped the call to "canonicalize". We can get rid of this wrapper. Unfortunately, it is not possible to just export "canonicalize" since this will conflict with a method signature from the math library, at least on modern Linuxes. So I decided to call the method JDK_Canonicalize and will correctly define it in jdk_util.h which can be included everywhere. I think this change is okay. My main concern
RE: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument
Hi David, thanks for your investigation. I'll prepare a fix to move back getPrefixed into canonicalize_md.c. However, could you please still fix your internal build in terms that it would have 'jdk_util.h' in the include path? Thanks Christoph > -Original Message- > From: David Holmes > Sent: Montag, 25. November 2019 01:02 > To: Langer, Christoph ; Alan Bateman > ; gerard ziemski > Cc: core-libs-dev@openjdk.java.net; hotspot-runtime- > d...@openjdk.java.net > Subject: Re: RFR: 8234185: Cleanup usage of canonicalize function between > libjava, hotspot and libinstrument > > > > On 25/11/2019 8:45 am, David Holmes wrote: > > On 25/11/2019 7:49 am, David Holmes wrote: > >> On 25/11/2019 7:33 am, David Holmes wrote: > >>> Hi Christoph, > >>> > >>> On 23/11/2019 12:04 am, Langer, Christoph wrote: > >>>> Hi, > >>>> > >>>> I'd like to push this change. However, running it through jdk-submit > >>>> shows reproducible errors: > >>>> > >>>> Job: mach5-one-clanger-JDK-8234185-1-20191122-0927-6913189 > >>>> BuildId: 2019-11-22-0926373.christoph.langer.source > >>>> No failed tests > >>>> Tasks Summary > >>>> • NA: 0 > >>>> • NOTHING_TO_RUN: 0 > >>>> • KILLED: 0 > >>>> • PASSED: 76 > >>>> • UNABLE_TO_RUN: 0 > >>>> • EXECUTED_WITH_FAILURE: 1 > >>>> • FAILED: 0 > >>>> • HARNESS_ERROR: 0 > >>>> Build > >>>> 1 Executed with failure > >>>> o windows-x64-install-windows-x64-build-19 error while building, > >>>> return value: 2 > >>>> > >>>> > >>>> Job: mach5-one-clanger-JDK-8234185-20191121-2313-6898791 > >>>> BuildId: 2019-11-21-2311357.christoph.langer.source > >>>> No failed tests > >>>> Tasks Summary > >>>> • NA: 0 > >>>> • NOTHING_TO_RUN: 0 > >>>> • KILLED: 0 > >>>> • PASSED: 76 > >>>> • UNABLE_TO_RUN: 0 > >>>> • EXECUTED_WITH_FAILURE: 1 > >>>> • FAILED: 0 > >>>> • HARNESS_ERROR: 0 > >>>> Build > >>>> 1 Executed with failure > >>>> o windows-x64-install-windows-x64-build-19 error while building, > >>>> return value: 2 > >>>> > >>>> > >>>> David already had a look and let me know that the following was the > >>>> reason: > >>>> > >>>> > t:/workspace/open/src/java.base/windows/native/libjava/canonicalize_md. > c(41): > >>>> fatal error C1083: Cannot open include file: 'jdk_util.h': No such > >>>> file or directory > >>>> > >>>> This is not explainable to me as I see this running through my local > >>>> build and our nightly builds without problems. I also can't explain > >>>> jdk_util.h can't be opened at this place - it should be there and > >>>> part of the include directories... > >>>> > >>>> I'd appreciate any help... > >>> > >>> I just dug a little deeper and this is failing in part of our closed > >>> build for the install repo. There is a library there that is using > >>> canonicalize_md.c directly - i.e. it adds that file to its source > >>> files list. The build instructions don't include that directory on > >>> the include directory list - hence the failure. But it will also fail > >>> due to the name change you made. > >> > >> Actually it appears that the other source code doesn't actually refer > >> to the canonicalize function at all, so a simple fix may be possible > >> at the build level on our side. I'm testing that now. > > > > It isn't the canonicalize function that is used, it is getPrefixed, > > which has now been moved to the io_util_md.c file. So a fix will be a > > bit more involved. > > I tried adding io_util_md.c to the library source list instead of > canonicalize_md.c but that just caused a slew of other compilation > failures, so I don't see any quick fix for us here. > > David > > > > > David > > > >> > >> David > >> - > >> > >>> Someone will need to work with you to make the necessary changes to > >>> our code.
Re: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument
On 25/11/2019 8:45 am, David Holmes wrote: On 25/11/2019 7:49 am, David Holmes wrote: On 25/11/2019 7:33 am, David Holmes wrote: Hi Christoph, On 23/11/2019 12:04 am, Langer, Christoph wrote: Hi, I'd like to push this change. However, running it through jdk-submit shows reproducible errors: Job: mach5-one-clanger-JDK-8234185-1-20191122-0927-6913189 BuildId: 2019-11-22-0926373.christoph.langer.source No failed tests Tasks Summary • NA: 0 • NOTHING_TO_RUN: 0 • KILLED: 0 • PASSED: 76 • UNABLE_TO_RUN: 0 • EXECUTED_WITH_FAILURE: 1 • FAILED: 0 • HARNESS_ERROR: 0 Build 1 Executed with failure o windows-x64-install-windows-x64-build-19 error while building, return value: 2 Job: mach5-one-clanger-JDK-8234185-20191121-2313-6898791 BuildId: 2019-11-21-2311357.christoph.langer.source No failed tests Tasks Summary • NA: 0 • NOTHING_TO_RUN: 0 • KILLED: 0 • PASSED: 76 • UNABLE_TO_RUN: 0 • EXECUTED_WITH_FAILURE: 1 • FAILED: 0 • HARNESS_ERROR: 0 Build 1 Executed with failure o windows-x64-install-windows-x64-build-19 error while building, return value: 2 David already had a look and let me know that the following was the reason: t:/workspace/open/src/java.base/windows/native/libjava/canonicalize_md.c(41): fatal error C1083: Cannot open include file: 'jdk_util.h': No such file or directory This is not explainable to me as I see this running through my local build and our nightly builds without problems. I also can't explain jdk_util.h can't be opened at this place - it should be there and part of the include directories... I'd appreciate any help... I just dug a little deeper and this is failing in part of our closed build for the install repo. There is a library there that is using canonicalize_md.c directly - i.e. it adds that file to its source files list. The build instructions don't include that directory on the include directory list - hence the failure. But it will also fail due to the name change you made. Actually it appears that the other source code doesn't actually refer to the canonicalize function at all, so a simple fix may be possible at the build level on our side. I'm testing that now. It isn't the canonicalize function that is used, it is getPrefixed, which has now been moved to the io_util_md.c file. So a fix will be a bit more involved. I tried adding io_util_md.c to the library source list instead of canonicalize_md.c but that just caused a slew of other compilation failures, so I don't see any quick fix for us here. David David David - Someone will need to work with you to make the necessary changes to our code. David Thanks Christoph -Original Message- From: Langer, Christoph Sent: Donnerstag, 21. November 2019 14:19 To: Alan Bateman ; core-libs- d...@openjdk.java.net; hotspot-runtime-...@openjdk.java.net Subject: RE: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument Hi Alan, thanks for the review. I'll push it then after running through jdk-submit. /Christoph -Original Message- From: Alan Bateman Sent: Donnerstag, 21. November 2019 09:51 To: Langer, Christoph ; core-libs- d...@openjdk.java.net; hotspot-runtime-...@openjdk.java.net Subject: Re: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument On 14/11/2019 15:37, Langer, Christoph wrote: Hi, please review this cleanup change regarding function "canonicalize" of libjava. Bug: https://bugs.openjdk.java.net/browse/JDK-8234185 Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8234185.0/ The goal is to cleanup how this function is defined and used. One thing is, that there was an unnecessary wrapper function "Canonicalize" in jni_util.c. It wrapped the call to "canonicalize". We can get rid of this wrapper. Unfortunately, it is not possible to just export "canonicalize" since this will conflict with a method signature from the math library, at least on modern Linuxes. So I decided to call the method JDK_Canonicalize and will correctly define it in jdk_util.h which can be included everywhere. I think this change is okay. My main concern when initially seeing this go by was that it would leak the \\?\ or \\?\UNC\ prefix into the canonical File when it wasn't there previously, this would of course have several implications. But I think you have it right and this is, as you position, just refactoring/cleanup. -Alan
Re: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument
On 25/11/2019 7:49 am, David Holmes wrote: On 25/11/2019 7:33 am, David Holmes wrote: Hi Christoph, On 23/11/2019 12:04 am, Langer, Christoph wrote: Hi, I'd like to push this change. However, running it through jdk-submit shows reproducible errors: Job: mach5-one-clanger-JDK-8234185-1-20191122-0927-6913189 BuildId: 2019-11-22-0926373.christoph.langer.source No failed tests Tasks Summary • NA: 0 • NOTHING_TO_RUN: 0 • KILLED: 0 • PASSED: 76 • UNABLE_TO_RUN: 0 • EXECUTED_WITH_FAILURE: 1 • FAILED: 0 • HARNESS_ERROR: 0 Build 1 Executed with failure o windows-x64-install-windows-x64-build-19 error while building, return value: 2 Job: mach5-one-clanger-JDK-8234185-20191121-2313-6898791 BuildId: 2019-11-21-2311357.christoph.langer.source No failed tests Tasks Summary • NA: 0 • NOTHING_TO_RUN: 0 • KILLED: 0 • PASSED: 76 • UNABLE_TO_RUN: 0 • EXECUTED_WITH_FAILURE: 1 • FAILED: 0 • HARNESS_ERROR: 0 Build 1 Executed with failure o windows-x64-install-windows-x64-build-19 error while building, return value: 2 David already had a look and let me know that the following was the reason: t:/workspace/open/src/java.base/windows/native/libjava/canonicalize_md.c(41): fatal error C1083: Cannot open include file: 'jdk_util.h': No such file or directory This is not explainable to me as I see this running through my local build and our nightly builds without problems. I also can't explain jdk_util.h can't be opened at this place - it should be there and part of the include directories... I'd appreciate any help... I just dug a little deeper and this is failing in part of our closed build for the install repo. There is a library there that is using canonicalize_md.c directly - i.e. it adds that file to its source files list. The build instructions don't include that directory on the include directory list - hence the failure. But it will also fail due to the name change you made. Actually it appears that the other source code doesn't actually refer to the canonicalize function at all, so a simple fix may be possible at the build level on our side. I'm testing that now. It isn't the canonicalize function that is used, it is getPrefixed, which has now been moved to the io_util_md.c file. So a fix will be a bit more involved. David David - Someone will need to work with you to make the necessary changes to our code. David Thanks Christoph -Original Message- From: Langer, Christoph Sent: Donnerstag, 21. November 2019 14:19 To: Alan Bateman ; core-libs- d...@openjdk.java.net; hotspot-runtime-...@openjdk.java.net Subject: RE: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument Hi Alan, thanks for the review. I'll push it then after running through jdk-submit. /Christoph -Original Message- From: Alan Bateman Sent: Donnerstag, 21. November 2019 09:51 To: Langer, Christoph ; core-libs- d...@openjdk.java.net; hotspot-runtime-...@openjdk.java.net Subject: Re: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument On 14/11/2019 15:37, Langer, Christoph wrote: Hi, please review this cleanup change regarding function "canonicalize" of libjava. Bug: https://bugs.openjdk.java.net/browse/JDK-8234185 Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8234185.0/ The goal is to cleanup how this function is defined and used. One thing is, that there was an unnecessary wrapper function "Canonicalize" in jni_util.c. It wrapped the call to "canonicalize". We can get rid of this wrapper. Unfortunately, it is not possible to just export "canonicalize" since this will conflict with a method signature from the math library, at least on modern Linuxes. So I decided to call the method JDK_Canonicalize and will correctly define it in jdk_util.h which can be included everywhere. I think this change is okay. My main concern when initially seeing this go by was that it would leak the \\?\ or \\?\UNC\ prefix into the canonical File when it wasn't there previously, this would of course have several implications. But I think you have it right and this is, as you position, just refactoring/cleanup. -Alan
Re: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument
On 25/11/2019 7:33 am, David Holmes wrote: Hi Christoph, On 23/11/2019 12:04 am, Langer, Christoph wrote: Hi, I'd like to push this change. However, running it through jdk-submit shows reproducible errors: Job: mach5-one-clanger-JDK-8234185-1-20191122-0927-6913189 BuildId: 2019-11-22-0926373.christoph.langer.source No failed tests Tasks Summary • NA: 0 • NOTHING_TO_RUN: 0 • KILLED: 0 • PASSED: 76 • UNABLE_TO_RUN: 0 • EXECUTED_WITH_FAILURE: 1 • FAILED: 0 • HARNESS_ERROR: 0 Build 1 Executed with failure o windows-x64-install-windows-x64-build-19 error while building, return value: 2 Job: mach5-one-clanger-JDK-8234185-20191121-2313-6898791 BuildId: 2019-11-21-2311357.christoph.langer.source No failed tests Tasks Summary • NA: 0 • NOTHING_TO_RUN: 0 • KILLED: 0 • PASSED: 76 • UNABLE_TO_RUN: 0 • EXECUTED_WITH_FAILURE: 1 • FAILED: 0 • HARNESS_ERROR: 0 Build 1 Executed with failure o windows-x64-install-windows-x64-build-19 error while building, return value: 2 David already had a look and let me know that the following was the reason: t:/workspace/open/src/java.base/windows/native/libjava/canonicalize_md.c(41): fatal error C1083: Cannot open include file: 'jdk_util.h': No such file or directory This is not explainable to me as I see this running through my local build and our nightly builds without problems. I also can't explain jdk_util.h can't be opened at this place - it should be there and part of the include directories... I'd appreciate any help... I just dug a little deeper and this is failing in part of our closed build for the install repo. There is a library there that is using canonicalize_md.c directly - i.e. it adds that file to its source files list. The build instructions don't include that directory on the include directory list - hence the failure. But it will also fail due to the name change you made. Actually it appears that the other source code doesn't actually refer to the canonicalize function at all, so a simple fix may be possible at the build level on our side. I'm testing that now. David - Someone will need to work with you to make the necessary changes to our code. David Thanks Christoph -Original Message- From: Langer, Christoph Sent: Donnerstag, 21. November 2019 14:19 To: Alan Bateman ; core-libs- d...@openjdk.java.net; hotspot-runtime-...@openjdk.java.net Subject: RE: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument Hi Alan, thanks for the review. I'll push it then after running through jdk-submit. /Christoph -Original Message- From: Alan Bateman Sent: Donnerstag, 21. November 2019 09:51 To: Langer, Christoph ; core-libs- d...@openjdk.java.net; hotspot-runtime-...@openjdk.java.net Subject: Re: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument On 14/11/2019 15:37, Langer, Christoph wrote: Hi, please review this cleanup change regarding function "canonicalize" of libjava. Bug: https://bugs.openjdk.java.net/browse/JDK-8234185 Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8234185.0/ The goal is to cleanup how this function is defined and used. One thing is, that there was an unnecessary wrapper function "Canonicalize" in jni_util.c. It wrapped the call to "canonicalize". We can get rid of this wrapper. Unfortunately, it is not possible to just export "canonicalize" since this will conflict with a method signature from the math library, at least on modern Linuxes. So I decided to call the method JDK_Canonicalize and will correctly define it in jdk_util.h which can be included everywhere. I think this change is okay. My main concern when initially seeing this go by was that it would leak the \\?\ or \\?\UNC\ prefix into the canonical File when it wasn't there previously, this would of course have several implications. But I think you have it right and this is, as you position, just refactoring/cleanup. -Alan
Re: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument
Hi Christoph, On 23/11/2019 12:04 am, Langer, Christoph wrote: Hi, I'd like to push this change. However, running it through jdk-submit shows reproducible errors: Job: mach5-one-clanger-JDK-8234185-1-20191122-0927-6913189 BuildId: 2019-11-22-0926373.christoph.langer.source No failed tests Tasks Summary • NA: 0 • NOTHING_TO_RUN: 0 • KILLED: 0 • PASSED: 76 • UNABLE_TO_RUN: 0 • EXECUTED_WITH_FAILURE: 1 • FAILED: 0 • HARNESS_ERROR: 0 Build 1 Executed with failure o windows-x64-install-windows-x64-build-19 error while building, return value: 2 Job: mach5-one-clanger-JDK-8234185-20191121-2313-6898791 BuildId: 2019-11-21-2311357.christoph.langer.source No failed tests Tasks Summary • NA: 0 • NOTHING_TO_RUN: 0 • KILLED: 0 • PASSED: 76 • UNABLE_TO_RUN: 0 • EXECUTED_WITH_FAILURE: 1 • FAILED: 0 • HARNESS_ERROR: 0 Build 1 Executed with failure o windows-x64-install-windows-x64-build-19 error while building, return value: 2 David already had a look and let me know that the following was the reason: t:/workspace/open/src/java.base/windows/native/libjava/canonicalize_md.c(41): fatal error C1083: Cannot open include file: 'jdk_util.h': No such file or directory This is not explainable to me as I see this running through my local build and our nightly builds without problems. I also can't explain jdk_util.h can't be opened at this place - it should be there and part of the include directories... I'd appreciate any help... I just dug a little deeper and this is failing in part of our closed build for the install repo. There is a library there that is using canonicalize_md.c directly - i.e. it adds that file to its source files list. The build instructions don't include that directory on the include directory list - hence the failure. But it will also fail due to the name change you made. Someone will need to work with you to make the necessary changes to our code. David Thanks Christoph -Original Message- From: Langer, Christoph Sent: Donnerstag, 21. November 2019 14:19 To: Alan Bateman ; core-libs- d...@openjdk.java.net; hotspot-runtime-...@openjdk.java.net Subject: RE: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument Hi Alan, thanks for the review. I'll push it then after running through jdk-submit. /Christoph -Original Message- From: Alan Bateman Sent: Donnerstag, 21. November 2019 09:51 To: Langer, Christoph ; core-libs- d...@openjdk.java.net; hotspot-runtime-...@openjdk.java.net Subject: Re: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument On 14/11/2019 15:37, Langer, Christoph wrote: Hi, please review this cleanup change regarding function "canonicalize" of libjava. Bug: https://bugs.openjdk.java.net/browse/JDK-8234185 Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8234185.0/ The goal is to cleanup how this function is defined and used. One thing is, that there was an unnecessary wrapper function "Canonicalize" in jni_util.c. It wrapped the call to "canonicalize". We can get rid of this wrapper. Unfortunately, it is not possible to just export "canonicalize" since this will conflict with a method signature from the math library, at least on modern Linuxes. So I decided to call the method JDK_Canonicalize and will correctly define it in jdk_util.h which can be included everywhere. I think this change is okay. My main concern when initially seeing this go by was that it would leak the \\?\ or \\?\UNC\ prefix into the canonical File when it wasn't there previously, this would of course have several implications. But I think you have it right and this is, as you position, just refactoring/cleanup. -Alan
RE: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument
Hi, I'd like to push this change. However, running it through jdk-submit shows reproducible errors: Job: mach5-one-clanger-JDK-8234185-1-20191122-0927-6913189 BuildId: 2019-11-22-0926373.christoph.langer.source No failed tests Tasks Summary • NA: 0 • NOTHING_TO_RUN: 0 • KILLED: 0 • PASSED: 76 • UNABLE_TO_RUN: 0 • EXECUTED_WITH_FAILURE: 1 • FAILED: 0 • HARNESS_ERROR: 0 Build 1 Executed with failure o windows-x64-install-windows-x64-build-19 error while building, return value: 2 Job: mach5-one-clanger-JDK-8234185-20191121-2313-6898791 BuildId: 2019-11-21-2311357.christoph.langer.source No failed tests Tasks Summary • NA: 0 • NOTHING_TO_RUN: 0 • KILLED: 0 • PASSED: 76 • UNABLE_TO_RUN: 0 • EXECUTED_WITH_FAILURE: 1 • FAILED: 0 • HARNESS_ERROR: 0 Build 1 Executed with failure o windows-x64-install-windows-x64-build-19 error while building, return value: 2 David already had a look and let me know that the following was the reason: t:/workspace/open/src/java.base/windows/native/libjava/canonicalize_md.c(41): fatal error C1083: Cannot open include file: 'jdk_util.h': No such file or directory This is not explainable to me as I see this running through my local build and our nightly builds without problems. I also can't explain jdk_util.h can't be opened at this place - it should be there and part of the include directories... I'd appreciate any help... Thanks Christoph > -Original Message- > From: Langer, Christoph > Sent: Donnerstag, 21. November 2019 14:19 > To: Alan Bateman ; core-libs- > d...@openjdk.java.net; hotspot-runtime-...@openjdk.java.net > Subject: RE: RFR: 8234185: Cleanup usage of canonicalize function between > libjava, hotspot and libinstrument > > Hi Alan, > > thanks for the review. I'll push it then after running through jdk-submit. > > /Christoph > > > -Original Message- > > From: Alan Bateman > > Sent: Donnerstag, 21. November 2019 09:51 > > To: Langer, Christoph ; core-libs- > > d...@openjdk.java.net; hotspot-runtime-...@openjdk.java.net > > Subject: Re: RFR: 8234185: Cleanup usage of canonicalize function between > > libjava, hotspot and libinstrument > > > > On 14/11/2019 15:37, Langer, Christoph wrote: > > > Hi, > > > > > > please review this cleanup change regarding function "canonicalize" of > > libjava. > > > > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8234185 > > > Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8234185.0/ > > > > > > > > > The goal is to cleanup how this function is defined and used. One thing > > > is, > > that there was an unnecessary wrapper function "Canonicalize" in jni_util.c. > > It wrapped the call to "canonicalize". We can get rid of this wrapper. > > Unfortunately, it is not possible to just export "canonicalize" since this > > will > > conflict with a method signature from the math library, at least on modern > > Linuxes. So I decided to call the method JDK_Canonicalize and will correctly > > define it in jdk_util.h which can be included everywhere. > > > > > I think this change is okay. My main concern when initially seeing this > > go by was that it would leak the \\?\ or \\?\UNC\ prefix into the > > canonical File when it wasn't there previously, this would of course > > have several implications. But I think you have it right and this is, as > > you position, just refactoring/cleanup. > > > > -Alan
RE: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument
Hi Alan, thanks for the review. I'll push it then after running through jdk-submit. /Christoph > -Original Message- > From: Alan Bateman > Sent: Donnerstag, 21. November 2019 09:51 > To: Langer, Christoph ; core-libs- > d...@openjdk.java.net; hotspot-runtime-...@openjdk.java.net > Subject: Re: RFR: 8234185: Cleanup usage of canonicalize function between > libjava, hotspot and libinstrument > > On 14/11/2019 15:37, Langer, Christoph wrote: > > Hi, > > > > please review this cleanup change regarding function "canonicalize" of > libjava. > > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8234185 > > Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8234185.0/ > > > > > > The goal is to cleanup how this function is defined and used. One thing is, > that there was an unnecessary wrapper function "Canonicalize" in jni_util.c. > It wrapped the call to "canonicalize". We can get rid of this wrapper. > Unfortunately, it is not possible to just export "canonicalize" since this > will > conflict with a method signature from the math library, at least on modern > Linuxes. So I decided to call the method JDK_Canonicalize and will correctly > define it in jdk_util.h which can be included everywhere. > > > I think this change is okay. My main concern when initially seeing this > go by was that it would leak the \\?\ or \\?\UNC\ prefix into the > canonical File when it wasn't there previously, this would of course > have several implications. But I think you have it right and this is, as > you position, just refactoring/cleanup. > > -Alan
Re: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument
On 14/11/2019 15:37, Langer, Christoph wrote: Hi, please review this cleanup change regarding function "canonicalize" of libjava. Bug: https://bugs.openjdk.java.net/browse/JDK-8234185 Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8234185.0/ The goal is to cleanup how this function is defined and used. One thing is, that there was an unnecessary wrapper function "Canonicalize" in jni_util.c. It wrapped the call to "canonicalize". We can get rid of this wrapper. Unfortunately, it is not possible to just export "canonicalize" since this will conflict with a method signature from the math library, at least on modern Linuxes. So I decided to call the method JDK_Canonicalize and will correctly define it in jdk_util.h which can be included everywhere. I think this change is okay. My main concern when initially seeing this go by was that it would leak the \\?\ or \\?\UNC\ prefix into the canonical File when it wasn't there previously, this would of course have several implications. But I think you have it right and this is, as you position, just refactoring/cleanup. -Alan
Re: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument
Thanks, Christoph! I forgot to tell that my recommendation is for the serviceability (j.l.instrument) coverage only. Thanks, Serguei On 11/19/19 23:47, Langer, Christoph wrote: Hi Serguei, thanks for the review. The patch successfully ran through our nightly test system which covers all these tests on several platforms. Best regards Christoph -Original Message- From: serguei.spit...@oracle.com Sent: Mittwoch, 20. November 2019 03:21 To: Langer, Christoph ; core-libs- d...@openjdk.java.net; hotspot-runtime-...@openjdk.java.net; gerard ziemski Subject: Re: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument Hi Christoph, The fix looks good to me. I'd recommend to run the jdk_instrument and vmTestbase_nsk_jvmti tests. Also, it can be safe to run: open/test/hotspot/jtreg/serviceability/jvmti open/test/hotspot/jtreg/runtime/cds/appcds open/test/hotspot/jtreg/runtime/BootClassAppendProp Thanks, Serguei On 11/14/19 07:37, Langer, Christoph wrote: Hi, please review this cleanup change regarding function "canonicalize" of libjava. Bug: https://bugs.openjdk.java.net/browse/JDK-8234185 Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8234185.0/ The goal is to cleanup how this function is defined and used. One thing is, that there was an unnecessary wrapper function "Canonicalize" in jni_util.c. It wrapped the call to "canonicalize". We can get rid of this wrapper. Unfortunately, it is not possible to just export "canonicalize" since this will conflict with a method signature from the math library, at least on modern Linuxes. So I decided to call the method JDK_Canonicalize and will correctly define it in jdk_util.h which can be included everywhere. Hotspot's classloader.cpp will dynamically resolve this method, so I add a local declaration of the function pointer in there. This change shall be predecessor of JDK-8223261, where a review was already started here: https://mail.openjdk.java.net/pipermail/core-libs- dev/2019-November/063398.html Thanks Christoph
RE: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument
Hi Serguei, thanks for the review. The patch successfully ran through our nightly test system which covers all these tests on several platforms. Best regards Christoph > -Original Message- > From: serguei.spit...@oracle.com > Sent: Mittwoch, 20. November 2019 03:21 > To: Langer, Christoph ; core-libs- > d...@openjdk.java.net; hotspot-runtime-...@openjdk.java.net; gerard > ziemski > Subject: Re: RFR: 8234185: Cleanup usage of canonicalize function between > libjava, hotspot and libinstrument > > Hi Christoph, > > The fix looks good to me. > I'd recommend to run the jdk_instrument and vmTestbase_nsk_jvmti tests. > Also, it can be safe to run: > open/test/hotspot/jtreg/serviceability/jvmti > open/test/hotspot/jtreg/runtime/cds/appcds > open/test/hotspot/jtreg/runtime/BootClassAppendProp > > Thanks, > Serguei > > On 11/14/19 07:37, Langer, Christoph wrote: > > Hi, > > > > please review this cleanup change regarding function "canonicalize" of > libjava. > > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8234185 > > Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8234185.0/ > > > > > > The goal is to cleanup how this function is defined and used. One thing is, > that there was an unnecessary wrapper function "Canonicalize" in jni_util.c. > It wrapped the call to "canonicalize". We can get rid of this wrapper. > Unfortunately, it is not possible to just export "canonicalize" since this > will > conflict with a method signature from the math library, at least on modern > Linuxes. So I decided to call the method JDK_Canonicalize and will correctly > define it in jdk_util.h which can be included everywhere. > > > > > > > > Hotspot's classloader.cpp will dynamically resolve this method, so I add a > local declaration of the function pointer in there. > > > > > > > > This change shall be predecessor of JDK-8223261, where a review was > already started here: https://mail.openjdk.java.net/pipermail/core-libs- > dev/2019-November/063398.html > > > > Thanks > > Christoph > >
Re: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument
Hi Christoph, The fix looks good to me. I'd recommend to run the jdk_instrument and vmTestbase_nsk_jvmti tests. Also, it can be safe to run: open/test/hotspot/jtreg/serviceability/jvmti open/test/hotspot/jtreg/runtime/cds/appcds open/test/hotspot/jtreg/runtime/BootClassAppendProp Thanks, Serguei On 11/14/19 07:37, Langer, Christoph wrote: Hi, please review this cleanup change regarding function "canonicalize" of libjava. Bug: https://bugs.openjdk.java.net/browse/JDK-8234185 Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8234185.0/ The goal is to cleanup how this function is defined and used. One thing is, that there was an unnecessary wrapper function "Canonicalize" in jni_util.c. It wrapped the call to "canonicalize". We can get rid of this wrapper. Unfortunately, it is not possible to just export "canonicalize" since this will conflict with a method signature from the math library, at least on modern Linuxes. So I decided to call the method JDK_Canonicalize and will correctly define it in jdk_util.h which can be included everywhere. Hotspot's classloader.cpp will dynamically resolve this method, so I add a local declaration of the function pointer in there. This change shall be predecessor of JDK-8223261, where a review was already started here: https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-November/063398.html Thanks Christoph
Re: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument
Guten Tag Langer, Christoph, am Montag, 18. November 2019 um 14:22 schrieben Sie: > I saw your other mail already but didn't find time to reply. Thanks, I wasn't sure if I'm received at all. Now that I know and because of your negative answer, I'm going to leave this thread alone and whoever is interested in discussing my problem might do so in the original thread: https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-November/063437.html Am only going to answer your concrete questions... > However, your case, the sporadic ERROR_NO_MORE_FILES, needs to be > understood first. I rather think if this happens, there's a real > condition for an IOException. I don't see much difference to real problems like ERROR_NETWORK_UNREACHABLE, in which case you don't even know if paths exist already, of what type they are etc. Compared to that, my ProcMon-logs looked pretty reasonable and "correct". The only difference is the error code some rare times, without any changes in the overall setup otherwise. > It should definitely be analyzed and > understood what the reason is for ERROR_NO_MORE_FILES. Are you aware > of other reports of this issue? No, I couldn't find anything of interest besides what I linked already on SO. Additionally, while my software is used by multiple customers with Windows environment, only one suffers from this problem and it seems to occur only in the recent past after migrating to Windows Server 2016, some new SAN etc. I'm somewhat sure that it didn't happen before, this would have been noticed, like it was now. > Was this already analyzed by some Windows experts, e.g. Microsoft > support? No and it's unlikely to happen, that customer is busy of course... I'm going to see if I can push things in that direction anyway. Mit freundlichen Grüßen, Thorsten Schöning -- Thorsten Schöning E-Mail: thorsten.schoen...@am-soft.de AM-SoFT IT-Systeme http://www.AM-SoFT.de/ Telefon...05151- 9468- 55 Fax...05151- 9468- 88 Mobil..0178-8 9468- 04 AM-SoFT GmbH IT-Systeme, Brandenburger Str. 7c, 31789 Hameln AG Hannover HRB 207 694 - Geschäftsführer: Andreas Muchow
RE: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument
Hi Thorsten, I saw your other mail already but didn't find time to reply. I'm actually not convinced that it is a good idea to add ERROR_NO_MORE_FILES to lastErrorReportable. The error codes listed there are conditions on which canonicalization of a path is stopped but the result is deemed correct. E.g. if the path only exists up to a certain directory, one can assume the rest of the path is canonic. Or if there are conditions like network errors or access denied, then further canonicalization isn't possible, too. However, your case, the sporadic ERROR_NO_MORE_FILES, needs to be understood first. I rather think if this happens, there's a real condition for an IOException. It should definitely be analyzed and understood what the reason is for ERROR_NO_MORE_FILES. Are you aware of other reports of this issue? Was this already analyzed by some Windows experts, e.g. Microsoft support? Best regards Christoph > -Original Message- > From: Thorsten Schöning > Sent: Montag, 18. November 2019 14:09 > To: core-libs-dev@openjdk.java.net > Cc: Langer, Christoph ; hotspot-runtime- > d...@openjdk.java.net; gerard ziemski > Subject: Re: RFR: 8234185: Cleanup usage of canonicalize function between > libjava, hotspot and libinstrument > > Guten Tag Langer, Christoph, > am Donnerstag, 14. November 2019 um 16:37 schrieben Sie: > > > please review this cleanup change regarding function "canonicalize" of > libjava. > [...] > > The goal is to cleanup how this function is defined and used.[...] > > If you are already changing "lastErrorReportable" for Windows, how > about adding ERROR_NO_MORE_FILES there as well to not run into > unnecessary exceptions under some circumstances? > > https://mail.openjdk.java.net/pipermail/core-libs-dev/2019- > November/063437.html > https://stackoverflow.com/questions/58825588/does-java-need-to- > support-error-no-more-files-when-canonicalizing-paths-on-windo > https://stackoverflow.com/questions/58825963/when-does-findfirstfilew- > set-last-error-to-be-error-no-more-files-instead-of-err?noredirect=1&lq=1 > > Mit freundlichen Grüßen, > > Thorsten Schöning > > -- > Thorsten Schöning E-Mail: thorsten.schoen...@am-soft.de > AM-SoFT IT-Systeme http://www.AM-SoFT.de/ > > Telefon...05151- 9468- 55 > Fax...05151- 9468- 88 > Mobil..0178-8 9468- 04 > > AM-SoFT GmbH IT-Systeme, Brandenburger Str. 7c, 31789 Hameln > AG Hannover HRB 207 694 - Geschäftsführer: Andreas Muchow
Re: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument
Guten Tag Langer, Christoph, am Donnerstag, 14. November 2019 um 16:37 schrieben Sie: > please review this cleanup change regarding function "canonicalize" of > libjava. [...] > The goal is to cleanup how this function is defined and used.[...] If you are already changing "lastErrorReportable" for Windows, how about adding ERROR_NO_MORE_FILES there as well to not run into unnecessary exceptions under some circumstances? https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-November/063437.html https://stackoverflow.com/questions/58825588/does-java-need-to-support-error-no-more-files-when-canonicalizing-paths-on-windo https://stackoverflow.com/questions/58825963/when-does-findfirstfilew-set-last-error-to-be-error-no-more-files-instead-of-err?noredirect=1&lq=1 Mit freundlichen Grüßen, Thorsten Schöning -- Thorsten Schöning E-Mail: thorsten.schoen...@am-soft.de AM-SoFT IT-Systeme http://www.AM-SoFT.de/ Telefon...05151- 9468- 55 Fax...05151- 9468- 88 Mobil..0178-8 9468- 04 AM-SoFT GmbH IT-Systeme, Brandenburger Str. 7c, 31789 Hameln AG Hannover HRB 207 694 - Geschäftsführer: Andreas Muchow
RE: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument
> I plan to review this change. We also need to figure out how to remove > the dependency on this function from the JPLIS agent as that should not > be there. Agree. I'd hope, however, that this can be done with a different change (unless you have an idea for a very simple, straightforward way that could fit under the umbrella of JDK- 8234185). /Christoph
Re: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument
On 18/11/2019 09:00, Langer, Christoph wrote: : Any other reviews (e.g. Gerard?) I plan to review this change. We also need to figure out how to remove the dependency on this function from the JPLIS agent as that should not be there. -Alan
RE: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument
Hi David, > This all seems fine to me. One clarification: Thanks for the review. > > - /* The appropriate location of getPrefixed() should be io_util_md.c, but > -java.lang.instrument package has hardwired canonicalize_md.c into their > -dll, to avoid complicate solution such as including io_util_md.c into > -that package, as a workaround we put this method here. > - */ > > I assume this hardwired usage was removed some time ago? AFAICS, yes. libinstrument builds/links against libjava. I cannot find any duplicates of canonicalize* in there. Any other reviews (e.g. Gerard?) Thanks & Best regards Christoph > > Thanks, > David > > On 15/11/2019 1:37 am, Langer, Christoph wrote: > > Hi, > > > > please review this cleanup change regarding function "canonicalize" of > libjava. > > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8234185 > > Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8234185.0/ > > > > > > The goal is to cleanup how this function is defined and used. One thing is, > that there was an unnecessary wrapper function "Canonicalize" in jni_util.c. > It wrapped the call to "canonicalize". We can get rid of this wrapper. > Unfortunately, it is not possible to just export "canonicalize" since this > will > conflict with a method signature from the math library, at least on modern > Linuxes. So I decided to call the method JDK_Canonicalize and will correctly > define it in jdk_util.h which can be included everywhere. > > > > > > > > Hotspot's classloader.cpp will dynamically resolve this method, so I add a > local declaration of the function pointer in there. > > > > > > > > This change shall be predecessor of JDK-8223261, where a review was > already started here: https://mail.openjdk.java.net/pipermail/core-libs- > dev/2019-November/063398.html > > > > Thanks > > Christoph > >
Re: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument
Hi Christoph, This all seems fine to me. One clarification: - /* The appropriate location of getPrefixed() should be io_util_md.c, but -java.lang.instrument package has hardwired canonicalize_md.c into their -dll, to avoid complicate solution such as including io_util_md.c into -that package, as a workaround we put this method here. - */ I assume this hardwired usage was removed some time ago? Thanks, David On 15/11/2019 1:37 am, Langer, Christoph wrote: Hi, please review this cleanup change regarding function "canonicalize" of libjava. Bug: https://bugs.openjdk.java.net/browse/JDK-8234185 Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8234185.0/ The goal is to cleanup how this function is defined and used. One thing is, that there was an unnecessary wrapper function "Canonicalize" in jni_util.c. It wrapped the call to "canonicalize". We can get rid of this wrapper. Unfortunately, it is not possible to just export "canonicalize" since this will conflict with a method signature from the math library, at least on modern Linuxes. So I decided to call the method JDK_Canonicalize and will correctly define it in jdk_util.h which can be included everywhere. Hotspot's classloader.cpp will dynamically resolve this method, so I add a local declaration of the function pointer in there. This change shall be predecessor of JDK-8223261, where a review was already started here: https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-November/063398.html Thanks Christoph
