Hi Yuri,
I don't see any consistency issue, since serialver and javap are intended for
different purposes.
I checked with a couple reviewers from earlier in this thread and they're ok
with the change as is, without the additional reflective code to pick up
modifiers from an existing declaration, so I've pushed the original change.
Thanks for the comments though.
s'marks
On 11/8/13 9:52 PM, Yuri Gaevsky wrote:
Stuart,
Sorry, but such inconsistency between serialver/javap is a bug (IMHO, of
course).
If there happens to be a declaration in the class that, probably mistakenly,
goes against this advice, serialver shouldn't emit a line that perpetuates this
mistake.
I would argue that for any real-life API it's almost impossible to fix such
explicitly-defined "bad" (i.e. public/protected) SVUID in any compatible way,
so emitting 'private' will not help there, unfortunately.
-Yuri
-----Original Message-----
From: Stuart Marks [mailto:stuart.ma...@oracle.com]
Sent: Friday, November 8, 2013 10:48 PM
To: Yuri Gaevsky
Cc: Alan Bateman; core-libs-dev
Subject: Re: 8028027: serialver should emit declaration with the 'private'
modifier
On 11/8/13 7:20 AM, Yuri Gaevsky wrote:
Well, it would be more consistent to check for existence of protected or public
serialVersionUID with Reflection API and change the serialver output
accordingly.
Please see suggested fix and its output below.
This change isn't consistent with the intent of the 'serialver' utility. Its intent is to produce a
declaration that's "suitable for copying into an evolving class." [1] (Clearly, this
means the source code of a class.) The spec [2] "strongly advises" that serialVersionUID
be private. As such, serialver should follow the strong advice given in the spec.
If there happens to be a declaration in the class that, probably mistakenly,
goes against this advice, serialver shouldn't emit a line that perpetuates this
mistake.
One can use "javap -v" to determine the presence of serialVersionUID field,
including its modifiers, if that's what's desired.
s'marks
[1] http://docs.oracle.com/javase/7/docs/technotes/tools/solaris/serialver.html
[2] http://docs.oracle.com/javase/7/docs/api/java/io/Serializable.html
Thanks,
-Yuri
$ serialver java.security.PublicKey
java.security.PublicKey: public static final long serialVersionUID =
7187392471159151072L;
$ serialver java.lang.Exception
java.lang.Exception: static final long serialVersionUID =
-3387516993124229948L;
$ serialver java.lang.AssertionError
java.lang.AssertionError: private static final long serialVersionUID =
-5013299493970297370L;
$ serialver javax.xml.ws.soap.SOAPFaultException
javax.xml.ws.soap.SOAPFaultException: private static final long
serialVersionUID = -104968645459360720L;
$ hg diff
diff --git a/src/share/classes/sun/tools/serialver/SerialVer.java
b/src/share/classes/sun/tools/serialver/SerialVer.java
--- a/src/share/classes/sun/tools/serialver/SerialVer.java
+++ b/src/share/classes/sun/tools/serialver/SerialVer.java
@@ -38,6 +38,7 @@
import java.net.MalformedURLException;
import java.util.StringTokenizer;
import sun.net.www.ParseUtil;
+import java.lang.reflect.Modifier;
public class SerialVer extends Applet {
GridBagLayout gb;
@@ -211,7 +212,17 @@
Class<?> cl = Class.forName(classname, false, loader);
ObjectStreamClass desc = ObjectStreamClass.lookup(cl);
if (desc != null) {
- return " static final long serialVersionUID = " +
+ String ams = "";
+ try {
+ final int mod =
+ cl.getDeclaredField("serialVersionUID").getModifiers();
+ ams = Modifier.isPublic(mod) ? "public"
+ : Modifier.isProtected(mod) ? "protected"
+ : Modifier.isPrivate(mod) ? "private" : "";
+ } catch (NoSuchFieldException nsfe) {
+ ams = "private";
+ }
+ return " " + ams + " static final long serialVersionUID = " +
desc.getSerialVersionUID() + "L;";
} else {
return null;
