Socket.java has some commented out code. I think it should just be
removed, right?
I'm guessing this is the only change to the previous webrev? If so, it
should be good to go. I don't think you need to re-generate a webrev for
the above trivial change.
-Chris
On 09/16/11 06:17 PM, Kurchi Hazra wrote:
Updated webrev:
http://cr.openjdk.java.net/~sherman/kurchi/7090158/webrev/
Thanks,
Kurchi
On 9/14/2011 12:42 PM, chris hegarty wrote:
Kurchi,
The problem here is that due to a bug in the compiler, CR 7090499, we
are not seeing raw type warnings for anonymous inner classes.
When Maurizio fixes this bug it is very likely that other areas of the
jdk where Sasha enabled -Werror will break the build.
If you like you can complete what you have in the networking area and
we can file a separate CR for this new issue. But we should make it a
priority.
Tom:
>....
>Or better, remove cl and change:
>
> clazz.getDeclaredMethod("connect", cl);
>
>to:
> clazz.getDeclaredMethod(
> "connect", SocketAddress.class, int.class
> );
Thanks Tom, this suggestion is much better. Kurchi we should probably
run with this.
-Chris.
On 14/09/2011 16:46, Kurchi Hazra wrote:
I remember having made this change as follows:
-bash-3.00$ hg diff src/share/classes/java/net/Socket.java
diff --git a/src/share/classes/java/net/Socket.java
b/src/share/classes/java/net/Socket.java
--- a/src/share/classes/java/net/Socket.java
+++ b/src/share/classes/java/net/Socket.java
@@ -459,10 +459,10 @@ class Socket implements java.io.Closeabl
oldImpl = AccessController.doPrivileged
(new PrivilegedAction<Boolean>() {
public Boolean run() {
- Class[] cl = new Class[2];
- cl[0] = SocketAddress.class;
- cl[1] = Integer.TYPE;
- Class clazz = impl.getClass();
+ Class<?>[] cl = new Class[2];
+ cl[0] = SocketAddress.class;
+ cl[1] = Integer.TYPE;
+ Class<?> clazz = impl.getClass();
while (true) {
try {
clazz.getDeclaredMethod("connect", cl);
I am not sure why webrev is not picking these changes, but these
warnings did show up.
Thanks,
Kurchi
On 9/14/2011 7:11 AM, Chris Hegarty wrote:
Maurizio and I noticed another issue in java.net.Socket
> Class<?>[] cl = new Class[2];
should generate a warning, but does not. Maurizio filed CR 7090499
against this. It is a compiler bug.
It should be: Class<?>[] cl = new Class<?>[2];
It is best that we fix this before pushing as it will break the build
if we wait until after 7090499 is fixed. Also we may be better off
waiting until after Maurizio runs a JPRT job with his fix for 7090499.
We may have other failures in areas where Sasha enabled warnings.
-Chris.
On 14/09/2011 14:17, Chris Hegarty wrote:
Thanks for reviewing this Max, I also went through the changes.
MessageHeader.filterAndAddHeaders() and
HttpURLConnection.getRequestproperties() also confused me. Essentially
this never work quite right. I had Maurizio ( javac guy ) look at the
old code with me and we figured out that the generic type on
filterAndAddHeaders was never enforced by the compiler as it was being
passed a raw Map.
The code in filterAndAddHeaders assumes that the values in the map are
Strings even though the type is declared as List<String>. Again this
cannot be enforced as the only client of filterAndAddHeaders is
passing
a raw Map. Anyway, it worked somehow!
What Kurchi has now looks right to me.
-Chris.
On 14/09/2011 06:22, Weijun Wang wrote:
On 09/14/2011 12:14 PM, Kurchi Hazra wrote:
Updated webrev :
http://cr.openjdk.java.net/~weijun/7090158/webrev.00/.
This should build correctly.
Yes, it does!
Some comments:
1. make/java/Makefile has no real change
2. make/javax/others/Makefile has only a new commented line
3. java/net/DatagramSocket.java and java/net/MulticastSocket.java
have
some real code changes around bind(). Maybe they should go to another
fix?
4. sun/net/www/MessageHeader.java:
231 for(Map.Entry<String, List<String>> entry : include.entrySet()) {
There should be a blank between "for" and "(", but probably not
necessary between "String," and "List" or "entry" and ":". You
decide.
234 l = new ArrayList<>();
Do we have a consensus on whether diamond can be used here? i.e.
assignment not on declaration.
Another thing:
I'm confused by the use of MessageHeader.filterAndAddHeaders() inside
HttpURLConnection.getRequestproperties() methods. Looking at the old
codes, it seems the userCookiesMap (in
HttpURLConnection.getRequestproperties) variable is only a
Map<String,String>, but the 2nd argument of filterAndAddHeaders() has
been declared as Map<String,List<String>> for some time, then again,
the
old filterAndAddHeaders() calls "l.add(entry.getValue())" which
suggests
the value of the map is still only String.
My current understanding is that both the old codes and Kurchi's code
changes work but the old one's method declaration is not correct.
Also,
if the include argument never contains multiple (or empty) string
values
for the same key, we can simply use Map<String,String>.
Thanks
Max
Thanks,
Kurchi
On 9/13/2011 7:55 PM, Weijun Wang wrote:
I apply the patch to my local repository and do a clean rebuild of
jdk-only. It shows 1 error and 92 warnings in javax and stopped.
Most
in src/share/classes/javax/xml/crypto/dsig and I remember Sean said
it's not easy to remove all warnings there because the codes are
shared between JDK and some Apache projects.
-Max
On 09/14/2011 03:13 AM, Alan Bateman wrote:
Kurchi Hazra wrote:
Something went wrong in the pasting. Can you check if this works
fine:
http://cr.openjdk.java.net/~chegar/7090158/webrev/
Yes, this webrev has what I expected to see.
-Alan
