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

[Frank Ding]

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