RFR (S): JDK-8222778: Packaging Tool (JEP 343) on Linux/AArch64
Dmitry Chuyko
dmitry.chuyko at bell-sw.com
Wed Jul 24 14:30:17 UTC 2019
Webrev without chdir() and write() changes:
http://cr.openjdk.java.net/~dchuyko/8222778/webrev.03/
When it is applied together with webrev.01 [1] from JDK-8228402, sandbox
is compiled and passes same jpackage tests on x86, aarch64 and other
platforms.
-Dmitry
[1] http://cr.openjdk.java.net/~herrick/8228402/
On 7/19/19 11:31 AM, Andrew Haley wrote:
> On 7/19/19 12:36 AM, Dmitry Chuyko wrote:
>> On 7/18/19 11:57 AM, Andrew Haley wrote:
>>> Hi,
>>>
>>> On 7/17/19 10:19 PM, Dmitry Chuyko wrote:
>>>> On 7/17/19 11:52 AM, Andrew Haley wrote:
>>>>> This is weird:
>>>>>
>>>>> --- old/src/jdk.jpackage/linux/native/libapplauncher/LinuxPlatform.cpp 2019-07-16 22:11:30.200258978 +0300
>>>>> +++ new/src/jdk.jpackage/linux/native/libapplauncher/LinuxPlatform.cpp 2019-07-16 22:11:29.867374851 +0300
>>>>> @@ -127,7 +127,7 @@
>>>>> }
>>>>>
>>>>> void LinuxPlatform::SetCurrentDirectory(TString Value) {
>>>>> - chdir(PlatformString(Value).toPlatformString());
>>>>> + int ignored = chdir(PlatformString(Value).toPlatformString());
>>>>> }
>>>>>
>>>>> Firstly it does not fix the problem: you've gone from an unused value to an
>>>>> unused variable. Secondly, we ignore the return value of printf all the time;
>>>>> what's different about this one?
>>>> chdir() is __wur, which is /usr/include/aarch64-linux-gnu/sys/cdefs.h:#
>>>> define __wur __attribute_warn_unused_result__
>>> OK, got it.
>>>
>>>> In some places this warning is disabled (CoreLibraries.gmk,
>>>> Awt2dLibraries.gmk).
>>>>
>>>> It would be more correct to handle the result here as chdir might not
>>>> succeeded in fact.
>>> Very much so. We shouldn't try to "shut up the compiler" in this way.Andrew Haley <aph at redhat.com>
>> I created a separate bug about chdir() and write() usages:
>> https://bugs.openjdk.java.net/browse/JDK-8228402
>>
>> Does it look good to silence warnings for platforms support changes?
> I think we should fix the bugs, not silence the warnings.
>
>> One
>> more option could be to exclude related changes from current review.
> OK.
>
More information about the core-libs-dev
mailing list