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.
