RFR(S): 8219142: Make libjimage strncpy uses GCC 8.x friendly

Mikael Vidstedt mikael.vidstedt at oracle.com
Sat Feb 16 00:39:16 UTC 2019


FWIW, this change, which removes ResourcePath etc. also passes tier1:

http://cr.openjdk.java.net/~mikael/webrevs/8219142/webrev.01/open/webrev/ <http://cr.openjdk.java.net/~mikael/webrevs/8219142/webrev.01/open/webrev/>

So if that function isn’t meant to be used from outside the JDK I’m pretty sure the code is indeed dead and should just be removed.

Cheers,
Mikael

> On Feb 15, 2019, at 2:39 PM, James Laskey <james.laskey at oracle.com> wrote:
> 
> The only user IIRC is the native class loader. It’s possible, due to the evolution of the API, that this entry is no longer used. Kill and verify. 
> 
> Sent from my iPhone
> 
>> On Feb 15, 2019, at 6:31 PM, Mikael Vidstedt <mikael.vidstedt at oracle.com> wrote:
>> 
>> 
>> This is interesting. I agree that the code should handle an overflow in a better way, throwing an exception seems like the reasonable thing to do. I started looking at doing exactly that.
>> 
>> However, that also made me have a look at how this code is actually used, and..
>> 
>> It’s not obvious that it actually is..
>> 
>> AFAICT, ImageFileReader::location_path is only used from JIMAGE_ResourcePath (in jimage.cpp). The only “user” I can find of that function in the JDK is in hotspot:
>> 
>> src/hotspot/share/classfile/classLoader.cpp:static JImage_ResourcePath_t           JImageResourcePath     = NULL;
>> src/hotspot/share/classfile/classLoader.cpp:  JImageResourcePath = CAST_TO_FN_PTR(JImage_ResourcePath_t, os::dll_lookup(handle, "JIMAGE_ResourcePath"));
>> src/hotspot/share/classfile/classLoader.cpp:  guarantee(JImageResourcePath != NULL, "function JIMAGE_ResourcePath not found”);
>> 
>> That’s the declaration of a variable, the initialization of it, and the check to make sure the lookup actually succeeded. However, the variable isn’t actually used anywhere after that.
>> 
>> Is this dead code which should just be removed instead? Or is JIMAGE_ResourcePath considered exported? Can (non-JDK) native code expect to find and use it?
>> 
>> Cheers,
>> Mikael
>> 
>>> On Feb 15, 2019, at 1:58 PM, James Laskey <james.laskey at oracle.com> wrote:
>>> 
>>> I wonder if you should flag overflow so no attempt is made to search with a bogus path. It’s not necessary but prevent future misunderstandings. 
>>> 
>>> Sent from my iPhone
>>> 
>>>> On Feb 15, 2019, at 5:24 PM, Mikael Vidstedt <mikael.vidstedt at oracle.com> 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.
>>>> 
>>>> Cheers,
>>>> Mikael
>>>> 
>>> 
>> 
> 



More information about the jigsaw-dev mailing list