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.