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

[Frank Ding]

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