Hi, You're right, I had removed the change to e_asin.c. 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. 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-dev@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-dev@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-dev@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-dev@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-...@openjdk.java.net>; serviceability-dev > (serviceability- > >>>>>>>>>> d...@openjdk.java.net) <serviceability-dev@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. > >>>>>
