RE: RFR: 8228482: fix xlc16/xlclang comparison of distinct pointer types and string literal conversion warnings
Hi Matthias,
I wouldn’t say „looks good”, but I think it’s the right thing to do 😊
The type casts look correct to fit to the AIX headers.
libodm_aix:
Good. Maybe we should open a new issue for freeing what’s returned by
odm_set_path and we could also remove the AIX 5 support.
NetworkInterface.c:
Strange, but ok. Should be reviewed by somebody else in addition.
Other files:
No comments.
Best regards,
Martin
From: ppc-aix-port-dev On Behalf Of
Baesken, Matthias
Sent: Dienstag, 23. Juli 2019 17:15
To: 'hotspot-...@openjdk.java.net' ;
core-libs-...@openjdk.java.net; net-dev@openjdk.java.net
Cc: 'ppc-aix-port-...@openjdk.java.net'
Subject: RFR: 8228482: fix xlc16/xlclang comparison of distinct pointer types
and string literal conversion warnings
Hello please review this patch .
It fixes a couple of xlc16/xlclang warnings , especially comparison of
distinct pointer types and string literal conversion warnings .
When building with xlc16/xlclang, we still have a couple of warnings that have
to be fixed :
warning: ISO C++11 does not allow conversion from string literal to 'char *'
[-Wwritable-strings]
for example :
/nightly/jdk/src/hotspot/os/aix/libodm_aix.cpp:81:18: warning: ISO C++11 does
not allow conversion from string literal to 'char *' [-Wwritable-strings]
odmWrapper odm("product", "/usr/lib/objrepos"); // could also use "lpp"
^
/nightly/jdk/src/hotspot/os/aix/libodm_aix.cpp:81:29: warning: ISO C++11 does
not allow conversion from string literal to 'char *' [-Wwritable-strings]
odmWrapper odm("product", "/usr/lib/objrepos"); // could also use "lpp"
^
warning: comparison of distinct pointer types, for example :
/nightly/jdk/src/java.desktop/aix/native/libawt/porting_aix.c:50:14: warning:
comparison of distinct pointer types ('void *' and 'char *')
[-Wcompare-distinct-pointer-types]
addr < (((char*)p->ldinfo_textorg) + p->ldinfo_textsize)) {
^ ~
Bug/webrev :
https://bugs.openjdk.java.net/browse/JDK-8228482
http://cr.openjdk.java.net/~mbaesken/webrevs/8228482.1/
Thanks, Matthias
Re: RFR 6563286 6797318 8177648 - Undeclared IAE thrown from HttpURLConnection.connect for some URLs
Hi Jaikiran, Thanks for looking at this issue. ProxySelector::select is not a particularly well specified method. The spec is a little vague on what is expected of the supplied URI parameter. It seems to me however, that the intent is that the supplied URI should contain a valid protocol (scheme) and host, even though that is perhaps not stated clearly enough. Though it does refer to the protocol and the destination address as the components it is looking for. The concern I have with the suggested fix is that it appears to contradict that implicit requirement. I wonder if another solution is possible where the IAE is caught at the appropriate place(s) and converted to an IOException which is what users are expecting. Michael. On 19/07/2019, 16:14, Jaikiran Pai wrote: Hello, Could I please get a review and a sponsor for a patch which fixes an issue in sun.net.spi.DefaultProxySelector? The patch is available as a webrev at http://cr.openjdk.java.net/~jpai/webrev/defaultproxyselector/1/webrev/ The issue has been reported in multiple different JBS issues[1][2][3] all coming back to the same root cause. There probably are other similar JBS issues related to this. The root cause is that the DefaultProxySelector throws IllegalArgumentException when the passed URI is non-null and thus contradicts its javadoc which (only) states the IllegalArgumentException is thrown when the URI is null. The change in the patch, now returns a Proxy.NO_PROXY when either the protocol or the host of the passed URI is null. This is the same as what's suggested in JDK-6797318. In addition to the change to DefaultProxySelector, this patch also changes sun.net.www.protocol.http.HttpURLConnection and sun.net.www.protocol.ftp.FtpURLConnection. Just before invoking the ProxySelector.select(...), both these classes use sun.net.www.ParseUtil.toURI(...) which can return null. The changes to these classes now ensure that the null isn't passed to the selector (to avoid a IllegalArgumentException) and instead is handled properly. 2 new test classes have been added in the patch. These tests fail (as expected) without the change to the source classes above and pass with these changes. After these changes, locally, I have also run the existing jtreg tests under test/jdk/java/net/HttpURLConnection and test/jdk/sun/net/www/http/HttpURLConnection/ and they have passed without regressions. I haven't added anything new for the FTP testing, since I didn't find a easy way to do that (based on my very limited search of existing tests). [1] https://bugs.openjdk.java.net/browse/JDK-6797318 [2] https://bugs.openjdk.java.net/browse/JDK-8177648 [3] https://bugs.openjdk.java.net/browse/JDK-6563286 -Jaikiran
