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

[Frank Ding]

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