On Mon, 30 Oct 2023 10:54:48 GMT, suchismith1993 <d...@openjdk.org> wrote:

> 1. Adding required compiler flags.
> 2. Adding required symbols.
> 
> JBS-ISSUE : [JDK-8317799](https://bugs.openjdk.org/browse/JDK-8317799)

Changes requested by mdoerr (Reviewer).

LGTM. You may want to replace the Copyright header of the new file. It was 
contributed by IBM.

Still good. I suggest to remove the empty lines at the beginning.

make/modules/java.base/Lib.gmk line 216:

> 214:     LDFLAGS := $(LDFLAGS_JDKLIB), \
> 215:     LDFLAGS_linux := -Wl$(COMMA)--no-as-needed, \
> 216:     LDFLAGS_aix := -brtl  -bloadmap:/home/hotspot/openjdk/symbol.log 
> -bE:/home/hotspot/tmp/all-libs/1.exp   , \

These files need to get added somewhere. Maybe symbols could get added to 
make/data/hotspot-symbols/symbols-aix? Or to new files in the same directory 
(with aix in the file names)?
Also, please remove extra whitespaces.

make/modules/java.base/Lib.gmk line 256:

> 254: endif
> 255: 
> 256: 
> ################################################################################

I guess this was done by mistake. Please revert.

src/java.base/share/native/libsyslookup/syslookup.c line 37:

> 35: 
> 36: 
> 37: 

Please avoid changing lines which you don't need to modify! Is `#include 
<math.h>` really needed, here? If so, please protect it by `#ifdef _AIX` and 
add a comment explaining why.

-------------

PR Review: https://git.openjdk.org/jdk/pull/16414#pullrequestreview-1705916269
Marked as reviewed by mdoerr (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16414#pullrequestreview-1716162496
PR Review: https://git.openjdk.org/jdk/pull/16414#pullrequestreview-1720838225
PR Review Comment: https://git.openjdk.org/jdk/pull/16414#discussion_r1377348834
PR Review Comment: https://git.openjdk.org/jdk/pull/16414#discussion_r1377352267
PR Review Comment: https://git.openjdk.org/jdk/pull/16414#discussion_r1377355378

Reply via email to