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

[Dmitry Samersoff]

Frank,

Excellent! Thank you for doing it.

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


-- 
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* Give Rabbit time, and he'll always get the answer