RFR(M): 8170663: Fix minor issues in corelib and servicabilty coding.

[Dmitry Samersoff]

Goetz,

This version of serviceability changes looks good to me.

-Dmitry

On 2016-12-07 18:26, 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/
> 
> Best regards,
>   Goetz.
> 
>> -----Original Message-----
>> From: Dmitry Samersoff [mailto:dmitry.samersoff at oracle.com]
>> Sent: Wednesday, December 07, 2016 2:43 PM
>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; Java Core Libs
>> <core-libs-dev at openjdk.java.net>; serviceability-dev (serviceability-
>> dev at openjdk.java.net) <serviceability-dev at 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.


-- 
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* I would love to change the world, but they won't give me the sources.