This changes fdlibm, which has historically been untouched. Don't commit without Joe Darcy's approval.
On Wed, Dec 7, 2016 at 7:26 AM, Lindenmaier, Goetz < goetz.lindenma...@sap.com> 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/ > > 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. >
