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

[Chris Hegarty]

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