I've updated the webrev. It includes all the changes we've discussed so far plus these:
http://cr.openjdk.java.net/~prappo/8048267/webrev.03/src/macosx/classes/sun/font/CStrike.java.sdiff.html http://cr.openjdk.java.net/~prappo/8048267/webrev.03/src/share/classes/com/sun/tools/example/debug/tty/BreakpointSpec.java.sdiff.html http://cr.openjdk.java.net/~prappo/8048267/webrev.03/src/share/classes/sun/reflect/UnsafeLongFieldAccessorImpl.java.sdiff.html http://cr.openjdk.java.net/~prappo/8048267/webrev.03/src/share/classes/sun/reflect/UnsafeQualifiedLongFieldAccessorImpl.java.sdiff.html http://cr.openjdk.java.net/~prappo/8048267/webrev.03/src/share/classes/sun/reflect/UnsafeQualifiedStaticLongFieldAccessorImpl.java.sdiff.html http://cr.openjdk.java.net/~prappo/8048267/webrev.03/src/share/classes/sun/reflect/UnsafeStaticLongFieldAccessorImpl.java.sdiff.html http://cr.openjdk.java.net/~prappo/8048267/webrev.03/src/solaris/classes/java/util/prefs/FileSystemPreferences.java.sdiff.html http://cr.openjdk.java.net/~prappo/8048267/webrev.03/src/solaris/classes/sun/awt/X11/XFocusProxyWindow.java.sdiff.html Andrej, about the 0L -> Long0 change. I think it's not necessary since this case is *maybe* different from others. There's even a comment on it: // Should never happen... but stay safe all the same. // else return 0L; Anyway at a runtime 0L and Long0 will be the same object. So don't worry about that. I think we're almost ready to go. Thanks, -Pavel On 28 Jun 2014, at 00:09, Otávio Gonçalves de Santana <otavioj...@java.net> wrote: > I found more two unnecessary valueOf. > About Andrej, is it not possible add two people in "Contributed-by:" tag? > > diff -r d02b062bc827 > src/share/classes/com/sun/tools/example/debug/tty/Commands.java > --- a/src/share/classes/com/sun/tools/example/debug/tty/Commands.java Fri Jun > 13 11:21:30 2014 -0700 > +++ b/src/share/classes/com/sun/tools/example/debug/tty/Commands.java Fri Jun > 27 20:06:28 2014 -0300 > @@ -935,7 +935,7 @@ > try { > methodInfo = loc.sourceName() + > MessageOutput.format("line number", > - new Object [] {new > Long(lineNumber)}); > + new Object [] {lineNumber}); > } catch (AbsentInformationException e) { > methodInfo = MessageOutput.format("unknown"); > } > @@ -946,7 +946,7 @@ > meth.declaringType().name(), > meth.name(), > methodInfo, > - new Long(pc)}); > + pc}); > } else { > MessageOutput.println("stack frame dump", > new Object [] {new Integer(frameNumber + > 1), > > > On Fri, Jun 27, 2014 at 5:16 PM, Andrej Golovnin <andrej.golov...@gmail.com> > wrote: > Hi Pavel, > > I'm not sure what the style guide for the source code says, but > there is a space between the cast operator and the field name in > src/share/classes/com/sun/jmx/snmp/daemon/SnmpAdaptorServer.java (line 881): > > @Override > public Long getSnmpOutGenErrs() { > - return new Long(snmpOutGenErrs); > + return (long) snmpOutGenErrs; > > } > > In all other changes there is no space after the cast operator. > > > > Also, I removed one useless creation of a Long object here: > > > > (line 191): > > > > http://cr.openjdk.java.net/~prappo/8048267/webrev.01/src/share/classes/sun/security/krb5/internal/KerberosTime.java.sdiff.html > > http://cr.openjdk.java.net/~prappo/8048267/webrev.02/src/share/classes/sun/security/krb5/internal/KerberosTime.java.sdiff.html > > And I have found one more :-) in KerberosTime.java: > > 252 public int getSeconds() { > 253 Long temp_long = kerberosTime / 1000L; > 254 return temp_long.intValue(); > 255 } > > This can be changed to: > > 252 public int getSeconds() { > 253 long temp_long = kerberosTime / 1000L; > 254 return (int) temp_long; > 255 } > > > > > > I wonder if we should leave a cast to int here: > > > > (line 383): > > > > http://cr.openjdk.java.net/~prappo/8048267/webrev.01/src/share/classes/sun/management/snmp/jvminstr/JvmMemoryImpl.java.sdiff.html > > http://cr.openjdk.java.net/~prappo/8048267/webrev.02/src/share/classes/sun/management/snmp/jvminstr/JvmMemoryImpl.java.sdiff.html > > > > Well it's probably nothing to worry about, but strictly speaking this > > changes the behaviour. Before the change, long was truncated to fit int. > > And now it's not. > > I would not change the behavior now. I think it is better to file a new issue > and change > it in a separate commit. Having this change in a separate commit may simplify > tracking > this change back in case it would cause some problems (I don't believe it). > And in the new issue you may provide better description for this change. > > And a minor comment for JvmMemoryImpl.java: > Maybe it is better to use Long0 in the line 387. So the code of the method > JvmMemoryImpl.getJvmMemoryPendingFinalCount() will look similar to the code > of other methods in the JvmMemoryImpl class. They all use the Long0 constant > to return 0L. > > In sun/management/snmp/jvminstr/JvmThreadingImpl.java in the line 313: > > 312 public Long getJvmThreadPeakCount() throws SnmpStatusException { > 313 return (long)getThreadMXBean().getPeakThreadCount(); > 314 } > > There is one space too much between "return" and the cast operator. The > additional space > was in the original version too, but maybe we can clean up here the code a > little bit. > > > > > P.S. Andrej, it looks like you don't have an 'Author' status. Well, that's > > a pity. We could mention you in the 'Reviewed-by' line. Your contributions > > are really good. > > Thanks! But I don't really care about it as long as I can help to improve the > overall code quality. > > Best regards, > Andrej Golovnin > > > > > -- > Atenciosamente. > > Otávio Gonçalves de Santana > > blog: http://otaviosantana.blogspot.com.br/ > twitter: http://twitter.com/otaviojava > site: http://www.otaviojava.com.br > (11) 98255-3513 >
