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

[Frank Ding]

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