On 9/12/2016 7:31 AM, Daniel D. Daugherty wrote:
On 12/8/16 1:59 PM, David Holmes wrote:
On 9/12/2016 12:21 AM, Lindenmaier, Goetz wrote:
Hi David,
thanks for looking at the change. New webrev:
http://cr.openjdk.java.net/~goetz/wr16/8170663-corlib_s11y/webrev.04/
src/java.base/share/native/libjli/java.c
As far as I can see the existing code is working perfectly fine.
Ah, thanks for the explanation, now I got it! Removed.
Again I'm not sure what the bug is here. On AIX the output string is
expanded with:
"%s/lib/%s/jli:"
I first edited this against jdk9/hs, where the arch is gone since
8066474,
http://hg.openjdk.java.net/jdk9/hs/jdk/rev/81508186e5bc
but the // was not adapted. Then I moved the change to jdk9/dev because
I thought I have to push it there. And yes, in that coding // would
be correct.
So I have to wait until hs is promoted ...
So just based on the current file, the change proposed - to remove one
/ - is not correct until the arch directory is removed.
That change is already in JDK9-hs:
Changeset: c14f9a7b4cab
Author: erikj
Date: 2016-12-05 17:55 +0100
URL: http://hg.openjdk.java.net/jdk9/hs/rev/c14f9a7b4cab
8066474: Remove the lib/ directory from Linux and Solaris images
Reviewed-by: tbell, ihse
along with changesets in the jdk and hotspot repos along with a few
closed repos.
Thanks Dan. So we need to see a webrev based on latest hs code - where
removing the extra / would be correct.
David
-----
Dan
David
-----
The jvmpath -> jvm_newpath change wasn't really necessary - a
comment on
the strdup would have sufficed IMO.
Dmitry asked me to add it. But I think it's ok.
Can I consider this reviewed now? I.e. could I push it once 8066474 is
promoted and Joe Darcy agreed?
Best regards,
Goetz.
-----Original Message-----
From: David Holmes [mailto:david.hol...@oracle.com]
Sent: Donnerstag, 8. Dezember 2016 09:14
To: Lindenmaier, Goetz <goetz.lindenma...@sap.com>; 'Dmitry Samersoff'
<dmitry.samers...@oracle.com>; Java Core Libs <core-libs-
d...@openjdk.java.net>; serviceability-dev (serviceability-
d...@openjdk.java.net) <serviceability-...@openjdk.java.net>
Subject: Re: RFR(M): 8170663: Fix minor issues in corelib and
servicabilty
coding.
Hi Goetz,
On 8/12/2016 1:26 AM, Lindenmaier, Goetz wrote:
Hi Dmitry,
yes, new_jvmpath is consistent with the other variables.
I also merged the ifs in SDE.c.
new webrev:
http://cr.openjdk.java.net/~goetz/wr16/8170663-corlib_s11y/webrev.03/
src/java.base/share/native/libjli/java.c
As far as I can see the existing code is working perfectly fine.
Given a
jvm.cfg with:
-server KNOWN
-client IGNORE
-myvm KNOWN
-oldvm ALIASED_TO -server
The usage text is:
-server to select the "server" VM
-myvm to select the "myvm" VM
-oldvm is a synonym for the "server" VM [deprecated]
The default VM is server,
because you are running on a server-class machine.
which is exactly what I would expect. Why do you think there is a bug?
---
src/java.base/unix/native/libjli/java_md_solinux.c
Again I'm not sure what the bug is here. On AIX the output string is
expanded with:
"%s/lib/%s/jli:"
so it needs to account for the extra "/lib//jli:" characters - and that
is exactly what the existing code does:
+ JLI_StrLen("/lib//jli:")
The jvmpath -> jvm_newpath change wasn't really necessary - a
comment on
the strdup would have sufficed IMO.
Thanks,
David
-----
Best regards,
Goetz.
-----Original Message-----
From: Dmitry Samersoff [mailto:dmitry.samers...@oracle.com]
Sent: Wednesday, December 07, 2016 2:43 PM
To: Lindenmaier, Goetz <goetz.lindenma...@sap.com>; Java Core Libs
<core-libs-dev@openjdk.java.net>; serviceability-dev (serviceability-
d...@openjdk.java.net) <serviceability-...@openjdk.java.net>
Subject: Re: RFR(M): 8170663: Fix minor issues in corelib and
servicabilty
coding.
Goetz,
SDE.c:
You might combine if at ll. 260 and 263 to one but it's just
matter of test.
if (sti == baseStratumIndex || sti < 0) {
return; /* Java stratum - return unchanged */
}
I'm not sure what you mean. I tried to fix it, but please
double-check the new webrev.
if cnt is <= 0 loop at l.267
for (; cnt-- > 0; ++fromEntry) {
is never run and we effectively do
*entryCountPtr = 0;
at l.283
So if we you suspect that cnt may become negative or 0:
(your v.01 changes)
260 if (sti < 0 && cnt > 0) {
261 return;
262 }
it's better to check it early.
But I'm not sure we have to care about negative/zero cnt here.
-Dmitry
On 2016-12-07 11:37, Lindenmaier, Goetz wrote:
Hi Dmitry,
thanks for looking at my change!
Updated webrev:
http://cr.openjdk.java.net/~goetz/wr16/8170663-corlib_s11y/webrev.02
* src/java.base/unix/native/libjli/java_md_solinux.c
Is this line correct?
519 jvmpath = JLI_StringDup(jvmpath);
It seems pointless. Should I remove it? (The whole file is a
horror.)
* src/jdk.jdwp.agent/share/native/libjdwp/SDE.c
It might be better to return immediately if cnt < 0,
regardless of value of sti.
I'm not sure what you mean. I tried to fix it, but please
double-check the new webrev.
* src/jdk.jdwp.agent/unix/native/libdt_socket/socket_md.c
It might be better to write
arg.l_linger = (on) ? (unsigned short)value.i : 0;
and leave one copy of setsockopt() call
Yes, looks better.
Best regards,
Goetz
-Dmitry
On 2016-12-06 16:12, Lindenmaier, Goetz wrote:
Hi,
This change fixes some minor issues found in our code scans.
I hope this correctly addresses corelib and serviceability issues.
Please review:
http://cr.openjdk.java.net/~goetz/wr16/8170663-
corlib_s11y/webrev.01/
Best regards,
Goetz.
Changes in detail:
e_asin.c
Code scan reports missing {}.
The code "if (huge+x>one) {" is only there to set the inexact
flag of
the processor.
It's a way to avoid the C compiler to optimize the code away.
It is
always true,
so the parenthesis of the outer else don't matter.
Although this basically just adds the {} I would like to submit
this to
avoid anybody else spends more the 30sec on understanding these
if statements.
k_standard.c
exc.retval is returned below and thus should always be
initialized.
imageDecompressor.cpp
Wrong destructor is used. Reported also by David CARLIER
java.c
in line 1865 'name' was used, it should be 'alias'.
java_md_solinux.c
"//" in path is useless. Further down a free is missing.
SDE.c
Call to stratumTableIndex can return negative value if
defaultStratumId
== null.
socket_md.c
arg.l_linger is passed to setsockopt uninitialized. Its use is
hidden in
the macros.
unpack.cpp
n.slice should not get negative argument for end, which is
passed from
dollar1.
But dollar1 can get negative where it is set to the result of
lastIndexOf(DOLLAR_MIN, DOLLAR_MAX, n, dollar2 - 1).
--
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* I would love to change the world, but they won't give me the
sources.
--
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* I would love to change the world, but they won't give me the
sources.
