src: fix exception message encoding#3288
Uh oh!
There was an error while loading. Please reload this page.
Conversation
mscdex added c++ 
mscdex commented Oct 8, 2015
evanlucas commented Oct 9, 2015
Is it not necessary to add on https://github.com/nodejs/node/blob/master/src/node.cc#L3210? |
src/node.cc Outdated
bnoordhuis commented Oct 9, 2015
This isn't complete yet, I think? There are a lot of (f)printf statements strewn throughout the code base. |
mscdex commented Oct 9, 2015
@evanlucas That has to do with program arguments, but the string values are not from user input such that they could contain utf8. @bnoordhuis Well, of the other |
mscdexforce-pushed the fix-exception-encoding branch 2 times, most recently from 3bdc6b3 to 5d7e67aComparemscdex commented Oct 9, 2015
silverwind commented Oct 12, 2015
CI is fine except the random ARM failures. I can confirm it fixes #3284. |
src/node.cc Outdated
There was a problem hiding this comment.
I'm kind of surprised this works, I thought vsnprintf() on Windows returned -1 when the output buffer is too small.
There was a problem hiding this comment.
I tested it on Windows 7 with VS2013 at least. I suppose _vscprintf() could be used instead...
bnoordhuis commented Oct 13, 2015
I'm going to defer to @nodejs/platform-windows. |
piscisaureus commented Oct 13, 2015
Lgtm |
The printf family of functions do not properly display UTF8 strings well on Windows. Use the appropriate wide character API instead if stderr is a tty. Fixes: nodejs#3284
mscdexforce-pushed the fix-exception-encoding branch from 5d7e67a to 238d1aeComparemscdex commented Oct 13, 2015
Final CI run after changing Only failures are an unrelated failure on Win10 and a recurring unrelated failure on FreeBSD 10.1 64-bit. |
The printf family of functions do not properly display UTF8 strings well on Windows. Use the appropriate wide character API instead if stderr is a tty. PR-URL: #3288Fixes: #3284 Reviewed-By: Bert Belder <[email protected]>
mscdex commented Oct 13, 2015
Landed in 2d35607. |
bergus commented Oct 13, 2015
Thanks for the quick fix! |
MylesBorins commented Oct 23, 2015
@jasnell should this be LTS? |
jasnell commented Oct 24, 2015
hmmm... we may need an LTS WG discussion on this one. |
jasnell commented Oct 26, 2015
@nodejs/lts ... should we land this one in v4.x? |
rvagg commented Oct 28, 2015
after looking at #3284 I would vote yes for this one, if this were just a "make it proper" fix then I'd be borderline but given this is fixing an actual user-reported bug it's much easier to justify because it's not academic |
The printf family of functions do not properly display UTF8 strings well on Windows. Use the appropriate wide character API instead if stderr is a tty. PR-URL: #3288Fixes: #3284 Reviewed-By: Bert Belder <[email protected]>
jasnell commented Oct 28, 2015
Landed in v4.x-staging in 9bffceb |
The printf family of functions do not properly display UTF8 strings well on Windows. Use the appropriate wide character API instead if stderr is a tty. PR-URL: #3288Fixes: #3284 Reviewed-By: Bert Belder <[email protected]>
10 participants
The printf family of functions do not properly display UTF8 strings well on Windows. Use the appropriate wide character API instead if stderr is a tty.
Fixes: #3284