Request for code review 6512101: Incorrect encoding in NetworkInterface.getDisplayName()

[Jonathan Lu]

Hi Frank,

Patch pushed @ http://hg.openjdk.java.net/jdk8/tl/jdk/rev/883feced1cdd

Best regards!
- Jonathan

On 12/11/2012 09:49 AM, Frank Ding wrote:
> Hi All,
>   Thank you all.  I will have Jonathan help to commit.
>
> Best regards,
> Frank
>
> On 12/10/2012 6:12 PM, Chris Hegarty wrote:
>> On 10/12/2012 08:01, Dmitry Samersoff wrote:
>>> Frank,
>>>
>>> Excellent! Thank you for doing it.
>>
>> Ditto, thanks Frank.
>>
>> I assume Sean or Neil will push this for you? Otherwise, just let me 
>> know and I can do it.
>>
>> -Chris.
>>
>>>
>>> -Dmitry
>>>
>>> On 2012-12-10 12:00, Frank Ding wrote:
>>>> Hi Dmitry,
>>>>    I updated wording accordingly @
>>>> http://cr.openjdk.java.net/~dingxmin/6512101/webrev.03.  It is now
>>>> changed to "Cannot get multibyte char for interface display name".  
>>>> What
>>>> about that?
>>>>
>>>> Best regards,
>>>> Frank
>>>>
>>>> On 12/10/2012 3:43 PM, Dmitry Samersoff wrote:
>>>>> Frank,
>>>>>
>>>>> Looks good for me.
>>>>>
>>>>> May be "Can't get WIDE string" should be changed to something more
>>>>> verbose.
>>>>>
>>>>> -Dmitry
>>>>>
>>>>>
>>>>> On 2012-12-10 11:40, Frank Ding wrote:
>>>>>> Hi Dmitry and Chris,
>>>>>>     Could you please review the second revision again?
>>>>>>
>>>>>> Thanks and Best regards,
>>>>>> Frank
>>>>>> On 11/29/2012 1:08 PM, Frank Ding wrote:
>>>>>>> Hi Dmitry and Chris,
>>>>>>>     Thanks for your comments.  With your comments incorporated, 
>>>>>>> I've
>>>>>>> prepared v2 @
>>>>>>> http://cr.openjdk.java.net/~dingxmin/6512101/webrev.02/. Could you
>>>>>>> please review it again?
>>>>>>>
>>>>>>> Best regards,
>>>>>>> Frank
>>>>>>>
>>>>>>> On 11/14/2012 12:12 AM, Chris Hegarty wrote:
>>>>>>>> On 11/11/2012 07:03 PM, Dmitry Samersoff wrote:
>>>>>>>>> Frank,
>>>>>>>>>
>>>>>>>>> Changes look good for me.
>>>>>>>> I admit that I am not an expert in this area, but given the
>>>>>>>> information you provided, and I guess you verified this in your
>>>>>>>> environment, the conversion would appear reasonable.
>>>>>>>>
>>>>>>>>> But it might be better to fall back to original behavior if
>>>>>>>>> MultiByteToWideChar return error, rather than abort.
>>>>>>>> I agree with Dmitry, fall back would be preferable. Can you 
>>>>>>>> make the
>>>>>>>> changes and post an updated webrev.
>>>>>>>>
>>>>>>>> -Chris.
>>>>>>>>
>>>>>>>>> -Dmitry
>>>>>>>>>
>>>>>>>>> On 2012-11-07 13:08, Frank Ding wrote:
>>>>>>>>>> Hi guys,
>>>>>>>>>>      Could you please take a look at patch below aimed to 
>>>>>>>>>> resolve
>>>>>>>>>> existing
>>>>>>>>>> bug http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6512101 ?
>>>>>>>>>> http://cr.openjdk.java.net/~dingxmin/6512101/webrev.01/
>>>>>>>>>>
>>>>>>>>>>      I happen to have a Chinese Win 7 environment. 
>>>>>>>>>> "buggy.png" is
>>>>>>>>>> current
>>>>>>>>>> output of test case described in bug system whereas "fixed.png"
>>>>>>>>>> is the
>>>>>>>>>> output after the my patch is applied.
>>>>>>>>>>
>>>>>>>>>> http://cr.openjdk.java.net/~dingxmin/6512101/buggy.png
>>>>>>>>>> http://cr.openjdk.java.net/~dingxmin/6512101/fixed.png
>>>>>>>>>>
>>>>>>>>>>      The patch simply converts to wide chars encoded in 
>>>>>>>>>> CP_OEMCP by
>>>>>>>>>> calling
>>>>>>>>>> MultiByteToWideChar.  We have confirmed a guy from Microsoft who
>>>>>>>>>> said that
>>>>>>>>>>
>>>>>>>>>> BEGIN QUOTE
>>>>>>>>>> I'm not sure how common it is to call the Java code that 
>>>>>>>>>> results in
>>>>>>>>>> calling the GetIfTable API but I would guess it does not 
>>>>>>>>>> happen that
>>>>>>>>>> often. Additionally, if it's rare that the adapter contains the
>>>>>>>>>> accented
>>>>>>>>>> characters, it would definitely be quite easy to miss in 
>>>>>>>>>> testing.
>>>>>>>>>>
>>>>>>>>>> I have not found any documentation about the encoding of the 
>>>>>>>>>> bDescr
>>>>>>>>>> string unfortunately. I did, however, debug through the API and
>>>>>>>>>> located
>>>>>>>>>> the place where it is generated. It is getting converted from a
>>>>>>>>>> UTF-16
>>>>>>>>>> string to a single-byte string using a conversion like this:
>>>>>>>>>>
>>>>>>>>>> WideCharToMultiByte(
>>>>>>>>>> CP_OEMCP,
>>>>>>>>>> WC_NO_BEST_FIT_CHARS,
>>>>>>>>>> <source string>,
>>>>>>>>>> -1,
>>>>>>>>>> IfRow->bDescr,
>>>>>>>>>> <size>,
>>>>>>>>>> NULL,
>>>>>>>>>> NULL);
>>>>>>>>>>
>>>>>>>>>> I have checked the source for Windows Vista, 2008, Windows 7, 
>>>>>>>>>> and
>>>>>>>>>> Windows 2008 R2. It is using CP_OEMCP in all of them. So 
>>>>>>>>>> using the
>>>>>>>>>> reverse conversion in your code using CP_OEMCP should be safe.
>>>>>>>>>> Alternatively, you can use the GetIfTable2 function
>>>>>>>>>> (http://msdn.microsoft.com/en-us/library/windows/desktop/aa365945^(v=vs.85).aspx 
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> ) which returns the same information in the original UTF-16
>>>>>>>>>> encoding.
>>>>>>>>>> END QUOTE
>>>>>>>>>>
>>>>>>>>>>      The link below may be helpful to the second param of
>>>>>>>>>> WideCharToMultiByte.
>>>>>>>>>> http://blogs.msdn.com/b/oldnewthing/archive/2012/05/04/10300670.aspx 
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> You comments are appreciated.
>>>>>>>>>> Best regards,
>>>>>>>>>> Frank
>>>>>>>>>>
>>>>>>>>>
>>>>>
>>>>
>>>
>>>
>>
>