Code Review Request: 7090158 Networking Libraries don't build with javac -Werror

[Kurchi Hazra]

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
>>>>

-- 
-Kurchi