On Feb 26, 2012, at 7:57 PM, James Melvin wrote: > Hi Stephen, > > The scope of the work for 7130404 is to... > > 1) Convert os.arch callsites to use 'x86_64' instead of 'amd64' > 2) Add *missing* os.name callsites for Mac OS X > > I did not originally plan to fixup *existing* os.name callsites. > However, your review has inspired me to expand the scope of 7130404 to > do just that. I walked the ~300 os.name/os.arch callsites and it turns > out that only a couple dozen need fixing up to use .contains() instead > of .startsWith(). I've updated the webrevs to reflect this additional > work. Might as well get it over now and get on to bigger and better > things. :) > > WEBREV: > http://cr.openjdk.java.net/~jmelvin/7130404/hotspot/webrev.03
Looks fine. agent/src/share/classes/sun/jvm/hotspot/utilities/PlatformInfo.java mentions "Darwin", not sure why. Also looks for "FreeBSD" "NetBSD" and "OpenBSD" But the rest of the jdk doesn't. Just seems strange to me to be looking for values of the Java property "os.name" that we don't appear to support. > http://cr.openjdk.java.net/~jmelvin/7130404/jdk/webrev.04 Again, looks fine. Also see a "Darwin" in src/solaris/classes/sun/nio/fs/DefaultFileSystemProvider.java: osname.equals("Darwin") I don't think it will ever say "Darwin" will it? Not your change, I'm just curious. And if it ever did say "Darwin", I doubt any of the other files that don't look for "Darwin" would work anyway. > http://cr.openjdk.java.net/~jmelvin/7130404/corba/webrev.02 Looks fine. > > TESTING: > JPRT hotspot and jdk builds and smoke tests on all platforms > Notepad, SwingSet2, SPECjbb2005 on Mac OS X > > Any final comments would be appreciated. Just my comments on Darwin, but don't consider that a hold up on the changes. -kto > > - Jim > > > > > On 2/22/12 7:39 PM, Stephen Bannasch wrote: >> At 6:10 PM -0500 2/22/12, James Melvin wrote: >>> Updated to include latest comments... >>> >>> http://cr.openjdk.java.net/~jmelvin/7130404/hotspot/webrev.02 >> >> line 46 still uses: os.startsWith("Mac OS X") instead of os.contains("OS X") >> >> http://cr.openjdk.java.net/~jmelvin/7130404/hotspot/webrev.02/agent/src/share/classes/sun/jvm/hotspot/utilities/PlatformInfo.java.html
