RFR(S): 8219142: Make libjimage strncpy uses GCC 8.x friendly
Mikael Vidstedt
mikael.vidstedt at oracle.com
Wed Feb 20 18:05:49 UTC 2019
Thanks for the review(s)!
Alan confirmed that the function/interface in question is JDK internal. Change pushed.
Cheers,
Mikael
> On Feb 19, 2019, at 8:03 PM, David Holmes <david.holmes at oracle.com> wrote:
>
> Hi Mikael,
>
> On 20/02/2019 9:10 am, Mikael Vidstedt wrote:
>> Sorry, here’s the webrev with the code in question removed:
>> http://cr.openjdk.java.net/~mikael/webrevs/8219142/webrev.01/open/webrev/ <http://cr.openjdk.java.net/~mikael/webrevs/8219142/webrev.01/open/webrev/>
>
> Hotspot code removal seems fine.
>
> A query on jimage.hpp though - is this an exported interface? (It has a strange copyright!) If so then you can't just delete an API, and the change would need a CSR request.
>
> Thanks,
> David
>
>> Cheers,
>> Mikael
>>> On Feb 19, 2019, at 1:12 PM, coleen.phillimore at oracle.com wrote:
>>>
>>>
>>>
>>> On 2/19/19 2:48 PM, Mikael Vidstedt wrote:
>>>>
>>>>> On Feb 18, 2019, at 12:03 AM, Alan Bateman <alan.bateman at oracle.com> wrote:
>>>>>
>>>>> On 15/02/2019 21:24, Mikael Vidstedt wrote:
>>>>>> Please review this change which addresses some warnings generated by GCC 8.2 related to the uses of strncpy in libjimage/imageFile.cpp.
>>>>>>
>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8219142
>>>>>> Webrev: http://cr.openjdk.java.net/~mikael/webrevs/8219142/webrev.00/open/webrev/ <http://cr.openjdk.java.net/~mikael/webrevs/8219142/webrev.00/open/webrev/>
>>>>>>
>>>>>>
>>>>>> In addition to feedback on the change itself, I’m taking suggestions on what tests to run. An earlier version of the change passed the typical tier1 testing. I’m going to run tier1 on this version as well, but let me know if there are additional tests I should run.
>>>>>>
>>>>> The jrtfs tests cover this area, the path to specify to jtreg is jdk/jdk/internal/jrtfs.
>>>> Thanks! I ran the jrtfs tests (all of tier2 even) and they all pass.
>>>>
>>>>> I skimmed through the changes and all red looks good :-) I assume the bug description can be changed as it's now about removing unused jimage functions rather than changes to works with a newer version of gcc.
>>>> Updated the summary to reflect the new charter of the enhancement.
>>>>
>>>> Can I please get a review from somebody in the runtime team as well for the hotspot changes?
>>>
>>> I don't see any hotspot changes from the link above. The code you added is certainly an improvement though.
>>>
>>> Coleen
>>>
>>>>
>>>> Cheers,
>>>> Mikael
More information about the hotspot-runtime-dev
mailing list