> object to the change to k_standard.c for the same reason. Ok, I remove that too.
Best regards, Goetz. > -----Original Message----- > From: David Holmes [mailto:david.hol...@oracle.com] > Sent: Mittwoch, 14. Dezember 2016 12:29 > To: Lindenmaier, Goetz <goetz.lindenma...@sap.com>; > daniel.daughe...@oracle.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. > > On 14/12/2016 9:00 PM, Lindenmaier, Goetz wrote: > > Hi, > > > > You're right, I had removed the change to e_asin.c. > > You only pointed Joe to that file AFAICS. I would expect he would also > object to the change to k_standard.c for the same reason. > > > New webrev anyways: > > http://cr.openjdk.java.net/~goetz/wr16/8170663-corlib_s11y/webrev.05/ > > > > java_md_solinux.c adds JLI_MemFree(new_jvmpath) that was missing. > > Okay. Thanks. > > David > > > Best regards, > > Goetz. > > > > > >> -----Original Message----- > >> From: David Holmes [mailto:david.hol...@oracle.com] > >> Sent: Mittwoch, 14. Dezember 2016 11:42 > >> To: Lindenmaier, Goetz <goetz.lindenma...@sap.com>; > >> daniel.daughe...@oracle.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. > >> > >> On 14/12/2016 8:23 PM, Lindenmaier, Goetz wrote: > >>> Hi David, > >>> > >>> I found "8169317: [s390] Various minor bug fixes and adaptions." > >>> in jdk9/dev this morning, so I thought it has been promoted based > >>> on some older change. I waited for that quite a while because tests > >>> in jdk9/dev kept failing on s390. > >>> How can I get the information when what was promoted? This always > >>> is a wild guess for me ... as well as when what will be promoted :) > >> > >> Yeah there's no notification in the bug report of when hs pushes up to > >> dev. The changeset email is the only record of exactly when it happens. > >> > >>> Do you think the promotion will happen tonight, or will it take > >>> another week? > >> > >> hs goes through Pre-Integration Testing (PIT) before it is pushed to > >> dev. There have been delays in getting that started and so a delay in it > >> finishing and being analyzed. It may still be a couple of days. > >> > >>> I removed > >>> - JLI_StrLen(jrepath) + JLI_StrLen(arch) + > >>> JLI_StrLen("/lib//jli:") > + > >>> + JLI_StrLen(jrepath) + JLI_StrLen(arch) + > >>> JLI_StrLen("/lib/jli:") + > >>> from > >>> http://cr.openjdk.java.net/~goetz/wr16/8170663-corlib_s11y/webrev.04/ > >> > >> With that gone that file only has the jvmpath rename - which isn't > >> fixing anything. > >> > >> Joe also asked for the fdlibm changes to dropped. > >> > >> Thanks, > >> David > >> > >>> Best regards, > >>> Goetz. > >>> > >>> > >>> > >>>> -----Original Message----- > >>>> From: David Holmes [mailto:david.hol...@oracle.com] > >>>> Sent: Mittwoch, 14. Dezember 2016 11:04 > >>>> To: Lindenmaier, Goetz <goetz.lindenma...@sap.com>; > >>>> daniel.daughe...@oracle.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. > >>>> > >>>> On 14/12/2016 7:48 PM, Lindenmaier, Goetz wrote: > >>>>> Hi, > >>>>> > >>>>> 8066474 has not been promoted. > >>>> > >>>> hs has not been able to push up to dev yet. > >>>> > >>>>> I'll remove the strlen // fix for aix from this change and > >>>>> push it without it. I'd like to get this done. > >>>> > >>>> I've lost track of what is now left in this set of changes ?? > >>>> > >>>> David > >>>> > >>>>> Best regards, > >>>>> Goetz. > >>>>> > >>>>>> -----Original Message----- > >>>>>> From: David Holmes [mailto:david.hol...@oracle.com] > >>>>>> Sent: Donnerstag, 8. Dezember 2016 23:03 > >>>>>> To: daniel.daughe...@oracle.com; 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. > >>>>>> > >>>>>> 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. > >>>>>>>