RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument

David Holmes david.holmes at oracle.com
Tue Dec 3 02:12:46 UTC 2019 

Hi Christoph,

Can you please post your updated patch for review and I will use it to 
check the fix for our internal build. Once you have your fix reviewed I 
will push both fixes together.

Thanks,
David

On 25/11/2019 10:41 pm, David Holmes wrote:
> Hi Christoph,
> 
> On 25/11/2019 10:38 pm, Langer, Christoph wrote:
>> Hi David,
>>
>> thanks for your investigation. I'll prepare a fix to move back 
>> getPrefixed into canonicalize_md.c. However, could you please still 
>> fix your internal build in terms that it would have 'jdk_util.h' in 
>> the include path?
> 
> That should be simple enough to do.
> 
> David
> 
>> Thanks
>> Christoph
>>
>>> -----Original Message-----
>>> From: David Holmes <david.holmes at oracle.com>
>>> Sent: Montag, 25. November 2019 01:02
>>> To: Langer, Christoph <christoph.langer at sap.com>; Alan Bateman
>>> <Alan.Bateman at oracle.com>; gerard ziemski <gerard.ziemski at oracle.com>
>>> Cc: core-libs-dev at openjdk.java.net; hotspot-runtime-
>>> dev at openjdk.java.net
>>> Subject: Re: RFR: 8234185: Cleanup usage of canonicalize function 
>>> between
>>> libjava, hotspot and libinstrument
>>>
>>>
>>>
>>> On 25/11/2019 8:45 am, David Holmes wrote:
>>>> On 25/11/2019 7:49 am, David Holmes wrote:
>>>>> On 25/11/2019 7:33 am, David Holmes wrote:
>>>>>> Hi Christoph,
>>>>>>
>>>>>> On 23/11/2019 12:04 am, Langer, Christoph wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> I'd like to push this change. However, running it through jdk-submit
>>>>>>> shows reproducible errors:
>>>>>>>
>>>>>>> Job: mach5-one-clanger-JDK-8234185-1-20191122-0927-6913189
>>>>>>> BuildId: 2019-11-22-0926373.christoph.langer.source
>>>>>>> No failed tests
>>>>>>> Tasks Summary
>>>>>>> •    NA: 0
>>>>>>> •    NOTHING_TO_RUN: 0
>>>>>>> •    KILLED: 0
>>>>>>> •    PASSED: 76
>>>>>>> •    UNABLE_TO_RUN: 0
>>>>>>> •    EXECUTED_WITH_FAILURE: 1
>>>>>>> •    FAILED: 0
>>>>>>> •    HARNESS_ERROR: 0
>>>>>>> Build
>>>>>>> 1 Executed with failure
>>>>>>> o    windows-x64-install-windows-x64-build-19 error while building,
>>>>>>> return value: 2
>>>>>>>
>>>>>>>
>>>>>>> Job: mach5-one-clanger-JDK-8234185-20191121-2313-6898791
>>>>>>> BuildId: 2019-11-21-2311357.christoph.langer.source
>>>>>>> No failed tests
>>>>>>> Tasks Summary
>>>>>>> •    NA: 0
>>>>>>> •    NOTHING_TO_RUN: 0
>>>>>>> •    KILLED: 0
>>>>>>> •    PASSED: 76
>>>>>>> •    UNABLE_TO_RUN: 0
>>>>>>> •    EXECUTED_WITH_FAILURE: 1
>>>>>>> •    FAILED: 0
>>>>>>> •    HARNESS_ERROR: 0
>>>>>>> Build
>>>>>>> 1 Executed with failure
>>>>>>> o    windows-x64-install-windows-x64-build-19 error while building,
>>>>>>> return value: 2
>>>>>>>
>>>>>>>
>>>>>>> David already had a look and let me know that the following was the
>>>>>>> reason:
>>>>>>>
>>>>>>>
>>> t:/workspace/open/src/java.base/windows/native/libjava/canonicalize_md.
>>> c(41):
>>>>>>> fatal error C1083: Cannot open include file: 'jdk_util.h': No such
>>>>>>> file or directory
>>>>>>>
>>>>>>> This is not explainable to me as I see this running through my local
>>>>>>> build and our nightly builds without problems. I also can't explain
>>>>>>> jdk_util.h can't be opened at this place - it should be there and
>>>>>>> part of the include directories...
>>>>>>>
>>>>>>> I'd appreciate any help...
>>>>>>
>>>>>> I just dug a little deeper and this is failing in part of our closed
>>>>>> build for the install repo. There is a library there that is using
>>>>>> canonicalize_md.c directly - i.e. it adds that file to its source
>>>>>> files list. The build instructions don't include that directory on
>>>>>> the include directory list - hence the failure. But it will also fail
>>>>>> due to the name change you made.
>>>>>
>>>>> Actually it appears that the other source code doesn't actually refer
>>>>> to the canonicalize function at all, so a simple fix may be possible
>>>>> at the build level on our side. I'm testing that now.
>>>>
>>>> It isn't the canonicalize function that is used, it is getPrefixed,
>>>> which has now been moved to the io_util_md.c file. So a fix will be a
>>>> bit more involved.
>>>
>>> I tried adding io_util_md.c to the library source list instead of
>>> canonicalize_md.c but that just caused a slew of other compilation
>>> failures, so I don't see any quick fix for us here.
>>>
>>> David
>>>
>>>>
>>>> David
>>>>
>>>>>
>>>>> David
>>>>> -----
>>>>>
>>>>>> Someone will need to work with you to make the necessary changes to
>>>>>> our code.
>>>>>>
>>>>>> David
>>>>>>
>>>>>>> Thanks
>>>>>>> Christoph
>>>>>>>
>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Langer, Christoph
>>>>>>>> Sent: Donnerstag, 21. November 2019 14:19
>>>>>>>> To: Alan Bateman <Alan.Bateman at oracle.com>; core-libs-
>>>>>>>> dev at openjdk.java.net; hotspot-runtime-dev at openjdk.java.net
>>>>>>>> Subject: RE: RFR: 8234185: Cleanup usage of canonicalize function
>>>>>>>> between
>>>>>>>> libjava, hotspot and libinstrument
>>>>>>>>
>>>>>>>> Hi Alan,
>>>>>>>>
>>>>>>>> thanks for the review. I'll push it then after running through
>>>>>>>> jdk-submit.
>>>>>>>>
>>>>>>>> /Christoph
>>>>>>>>
>>>>>>>>> -----Original Message-----
>>>>>>>>> From: Alan Bateman <Alan.Bateman at oracle.com>
>>>>>>>>> Sent: Donnerstag, 21. November 2019 09:51
>>>>>>>>> To: Langer, Christoph <christoph.langer at sap.com>; core-libs-
>>>>>>>>> dev at openjdk.java.net; hotspot-runtime-dev at openjdk.java.net
>>>>>>>>> Subject: Re: RFR: 8234185: Cleanup usage of canonicalize function
>>>>>>>>> between
>>>>>>>>> libjava, hotspot and libinstrument
>>>>>>>>>
>>>>>>>>> On 14/11/2019 15:37, Langer, Christoph wrote:
>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>> please review this cleanup change regarding function
>>>>>>>>>> "canonicalize" of
>>>>>>>>> libjava.
>>>>>>>>>>
>>>>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8234185
>>>>>>>>>> Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8234185.0/
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> The goal is to cleanup how this function is defined and used. One
>>>>>>>>>> thing is,
>>>>>>>>> that there was an unnecessary wrapper function "Canonicalize" in
>>>>>>>>> jni_util.c.
>>>>>>>>> It wrapped the call to "canonicalize". We can get rid of this
>>>>>>>>> wrapper.
>>>>>>>>> Unfortunately, it is not possible to just export "canonicalize"
>>>>>>>>> since this will
>>>>>>>>> conflict with a method signature from the math library, at least
>>>>>>>>> on modern
>>>>>>>>> Linuxes. So I decided to call the method JDK_Canonicalize and will
>>>>>>>>> correctly
>>>>>>>>> define it in jdk_util.h which can be included everywhere.
>>>>>>>>>>
>>>>>>>>> I think this change is okay. My main concern when initially seeing
>>>>>>>>> this
>>>>>>>>> go by was that it would leak the \\?\ or \\?\UNC\ prefix into the
>>>>>>>>> canonical File when it wasn't there previously, this would of 
>>>>>>>>> course
>>>>>>>>> have several implications. But I think you have it right and this
>>>>>>>>> is, as
>>>>>>>>> you position, just refactoring/cleanup.
>>>>>>>>>
>>>>>>>>> -Alan

More information about the core-libs-dev mailing list