> On Feb 15, 2019, at 2:31 PM, Mikael Vidstedt <mikael.vidst...@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.
Sorry, throwing an exception isn’t a reasonable thing to do, it would have to
be flagged some other way, however, the more important question is still: is
this code dead?
>
> 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.las...@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.vidst...@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
>>>
>>
>
