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

Frank Ding dingxmin at linux.vnet.ibm.com
Mon Dec 10 17:49:21 PST 2012


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




More information about the net-dev mailing list