On Wed, Sep 1, 2010 at 06:27, Michael McMahon <michael.x.mcma...@oracle.com>wrote:
> Martin Buchholz wrote: > >> Hi net maintainers, >> >> I'd like you to do a code review. >> >> I got frustrated with incomplete information from exceptions thrown from >> java.net <http://java.net>, so I hacked up some fixes for them. This is >> the result. >> >> >> generify; remove compiler warnings, typos, casts; return status >> information via gai_strerror when getaddrinfo fails >> >> http://cr.openjdk.java.net/~martin/webrevs/openjdk7/netErrors/<http://cr.openjdk.java.net/%7Emartin/webrevs/openjdk7/netErrors/>< >> http://cr.openjdk.java.net/%7Emartin/webrevs/openjdk7/netErrors/> >> >> But... I don't know what I'm doing here, and the underlying bug I was >> tracking down is fixed, so it's hard for me to test my code changes. It >> would be good if a java.net <http://java.net> maintainer would add a test >> case, or at least verify my exception improvements with an ad hoc test. >> >> Thanks, >> >> Martin >> > Hi Martin, > > Looks like a useful set of changes. I can confirm the exception text is > improved on Solaris and > Linux. I'm not sure there is much value in adding a regression test case > that checks for specific > exception text string though ... > > I try to do something like that in ProcessBuilder tests, but it can't be done very portably. } catch (IOException e) { String m = e.getMessage(); if (EnglishUnix.is() && ! matches(m, "Permission denied")) but I'm not going to do that here. > On the source change itself: > > InetAddress.java: I'd prefer to see the type parameters used in the places > where you > rely on jdk7 type inference. I don't think clarity is enhanced in these > cases, since the parameters > are quite straight forward. > As you wish. > > Variable "address" could be renamed "addresses" since we know now it refers > to an array. > > I wasn't going to do that, because that would trigger many other address => addresses changes. But since you asked, I went ahead and did that, and changed the names of some of the private methods. Webrev regenerated. Martin > Other than that, looks fine. > > - Michael >
