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 
<a...@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? One more option could be to exclude related changes from current review.


'arvmv7hl' is a common build environment/target for arm port and looks
like a compatible option. We may look into VM binary metadata though to
get exact build arch.
That would be wise.

getRpmArch() now asks rpmbuild to evaluate _target_cpu macro on current test host. That allows tests to find rpms that are created by the tool as it also uses default arch.

Updated webrev: http://cr.openjdk.java.net/~dchuyko/8222778/webrev.02/

-Dmitry


Reply via email to