Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(19)

#1602: windows console doesn't print utf8 (Py30a2)

Can't Edit
Can't Publish+Mail
Start Review
Created:
6 years, 10 months ago by mark
Modified:
1 year ago
Reviewers:
vadmium+py, eryksun+pybugs, steve.dower, berker.peksag
CC:
lemburg, mhammond, terry.reedy, pmoore, tzot, amaury.forgeotdarc, Nick Coghlan, AntoinePitrou, giampaolo.rodola, tim.golden, mark_qtrac.eu, ned.deily, cburgmer_ira.uka.de, ezio.melotti, v+python, hippytrail_gmail.com, flox, irdb, david-sarah_jacaranda.org, santa4nt, akira, camior_gmail.com, devnull_psf.upfronthosting.co.za, MerlinSchindlbeck_googlemail.com, lilydjwg_gmail.com, berkerpeksag, Martin Panter, piotr.dobrogost, eryksun, Drekin, steve.dower, schampailler_skynet.be, stijn.verstraeten_med.kuleuven.be, jonitis_gmail.com, chris_gurneeconsulting.net, escapewindow, dead1ne_gmail.com, davispuh
Visibility:
Public.

Patch Set 1 #

Patch Set 2 #

Patch Set 3 #

Patch Set 4 #

Total comments: 17

Patch Set 5 #

Total comments: 12

Patch Set 6 #

Total comments: 26
Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Doc/whatsnew/3.6.rst View 1 2 3 4 5 2 chunks +16 lines, -0 lines 4 comments Download
Include/pydebug.h View 1 2 3 4 5 1 chunk +4 lines, -0 lines 0 comments Download
Lib/test/test_os.py View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
Misc/NEWS View 1 2 3 4 5 2 chunks +2 lines, -1 line 0 comments Download
Modules/_io/_iomodule.c View 1 2 3 4 5 4 chunks +21 lines, -3 lines 0 comments Download
Modules/_io/_iomodule.h View 1 2 3 4 5 2 chunks +10 lines, -0 lines 0 comments Download
Modules/_io/clinic/winconsoleio.c.h View 1 2 3 4 5 1 chunk +330 lines, -0 lines 0 comments Download
Modules/_io/winconsoleio.c View 1 2 3 4 5 1 chunk +969 lines, -0 lines 18 comments Download
PCbuild/pythoncore.vcxproj View 1 2 3 4 5 1 chunk +1 line, -0 lines 0 comments Download
PCbuild/pythoncore.vcxproj.filters View 1 2 3 4 5 1 chunk +3 lines, -0 lines 0 comments Download
Parser/myreadline.c View 1 2 3 4 5 3 chunks +114 lines, -0 lines 4 comments Download
Python/pylifecycle.c View 1 2 3 4 5 5 chunks +22 lines, -0 lines 0 comments Download

Messages

Total messages: 12
Martin Panter
https://bugs.python.org/review/1602/diff/18128/Modules/_io/_iomodule.c File Modules/_io/_iomodule.c (right): https://bugs.python.org/review/1602/diff/18128/Modules/_io/_iomodule.c#newcode372 Modules/_io/_iomodule.c:372: encoding = "utf-16-le"; The indentation is all over the ...
1 year, 1 month ago #1
eryksun+pybugs_gmail.com
http://bugs.python.org/review/1602/diff/18280/Parser/myreadline.c File Parser/myreadline.c (right): http://bugs.python.org/review/1602/diff/18280/Parser/myreadline.c#newcode120 Parser/myreadline.c:120: if (!ReadConsoleW(hStdIn, &wbuf[n], chunk_size, &rn, NULL) || rn == ...
1 year ago #2
eryksun+pybugs_gmail.com
http://bugs.python.org/review/1602/diff/18280/Parser/myreadline.c File Parser/myreadline.c (right): http://bugs.python.org/review/1602/diff/18280/Parser/myreadline.c#newcode182 Parser/myreadline.c:182: fprintf(stderr, "%s", prompt); Note that this also writes the ...
1 year ago #3
steve.dower
http://bugs.python.org/review/1602/diff/18280/Parser/myreadline.c File Parser/myreadline.c (right): http://bugs.python.org/review/1602/diff/18280/Parser/myreadline.c#newcode120 Parser/myreadline.c:120: if (!ReadConsoleW(hStdIn, &wbuf[n], chunk_size, &rn, NULL) || rn == ...
1 year ago #4
eryksun+pybugs_gmail.com
http://bugs.python.org/review/1602/diff/18280/Parser/myreadline.c File Parser/myreadline.c (right): http://bugs.python.org/review/1602/diff/18280/Parser/myreadline.c#newcode143 Parser/myreadline.c:143: if (wbuf[0] == '\x1a') { I was going on ...
1 year ago #5
eryksun+pybugs_gmail.com
http://bugs.python.org/review/1602/diff/18280/Modules/_io/winconsoleio.c File Modules/_io/winconsoleio.c (right): http://bugs.python.org/review/1602/diff/18280/Modules/_io/winconsoleio.c#newcode85 Modules/_io/winconsoleio.c:85: HANDLE handle; Why don't you always wrap the handle ...
1 year ago #6
eryksun+pybugs_gmail.com
http://bugs.python.org/review/1602/diff/18367/Parser/myreadline.c File Parser/myreadline.c (right): http://bugs.python.org/review/1602/diff/18367/Parser/myreadline.c#newcode136 Parser/myreadline.c:136: HANDLE hInterruptEvent = _PyOS_SigintEvent(); This event also needs to ...
1 year ago #7
eryksun+pybugs_gmail.com
http://bugs.python.org/review/1602/diff/18367/Modules/_io/winconsoleio.c File Modules/_io/winconsoleio.c (right): http://bugs.python.org/review/1602/diff/18367/Modules/_io/winconsoleio.c#newcode290 Modules/_io/winconsoleio.c:290: DWORD access = GENERIC_READ; Both CONIN$ and CONOUT$ should ...
1 year ago #8
eryksun+pybugs_gmail.com
http://bugs.python.org/review/1602/diff/18367/Modules/_io/winconsoleio.c File Modules/_io/winconsoleio.c (right): http://bugs.python.org/review/1602/diff/18367/Modules/_io/winconsoleio.c#newcode491 Modules/_io/winconsoleio.c:491: wlen = 1; Of course that last sentence is ...
1 year ago #9
eryksun+pybugs_gmail.com
http://bugs.python.org/review/1602/diff/18367/Parser/myreadline.c File Parser/myreadline.c (right): http://bugs.python.org/review/1602/diff/18367/Parser/myreadline.c#newcode113 Parser/myreadline.c:113: static wchar_t wbuf_local[1024 * 32]; An initial size of ...
1 year ago #10
berkerpeksag
http://bugs.python.org/review/1602/diff/18367/Doc/whatsnew/3.6.rst File Doc/whatsnew/3.6.rst (right): http://bugs.python.org/review/1602/diff/18367/Doc/whatsnew/3.6.rst#newcode242 Doc/whatsnew/3.6.rst:242: provide correctly read str objects to Python code. ``sys.stdin``, ...
1 year ago #11
Martin Panter
1 year ago #12
https://bugs.python.org/review/1602/diff/18367/Modules/_io/winconsoleio.c
File Modules/_io/winconsoleio.c (right):

https://bugs.python.org/review/1602/diff/18367/Modules/_io/winconsoleio.c#new...
Modules/_io/winconsoleio.c:4: Classes defined here: WinConsoleReader,
WinConsoleWriter
There is no other mention of these names anywhere in 1602_6.patch

https://bugs.python.org/review/1602/diff/18367/Modules/_io/winconsoleio.c#new...
Modules/_io/winconsoleio.c:28: /* BUFSIZ determines how many characters can be
types at the console
typed?

But it only seems to determine SMALLCHUNK, which in turn determines how quickly
the buffer for readall() is expanded.

https://bugs.python.org/review/1602/diff/18367/Modules/_io/winconsoleio.c#new...
Modules/_io/winconsoleio.c:489: wlen = len / 4;
len is Py_ssize_t, which could be 64 bits, but wlen is DWORD, which is
apparently always 32 bits. So if you ask for exactly 16 GiB, I think it will do
a zero read. Not a disaster, but I think it should at least read one byte, and
ideally as many as practical.

https://bugs.python.org/review/1602/diff/18367/Modules/_io/winconsoleio.c#new...
Modules/_io/winconsoleio.c:500: if (len == read_len || wlen == 0)
This does not make much sense. The second condition will be true if original len
< 8 and buffered data was copied, or if len == 0.

The first condition will be true if exactly half of the requested data was
copied from the buffer. Unless it is possible to buffer four bytes, the first
condition seems redundant. Anyway, the whole thing looks suspicious. It could at
least do with an explanatory comment.

https://bugs.python.org/review/1602/diff/18367/Modules/_io/winconsoleio.c#new...
Modules/_io/winconsoleio.c:616: "than a Python bytes object can hold");
Technically it is more _characters_ than a DWORD can hold (I understand DWORD is
32 bits, bytes object may hold more on 64 bit system)

https://bugs.python.org/review/1602/diff/18367/Modules/_io/winconsoleio.c#new...
Modules/_io/winconsoleio.c:652: if (n == 0 || buf[len] == '\x1a')
read_console_w() overwrites 0x1A and sets n == 0, so I think the second
condition is a red herring

https://bugs.python.org/review/1602/diff/18367/Modules/_io/winconsoleio.c#new...
Modules/_io/winconsoleio.c:658: if (len > 0 && buf[0] == '\x1a') {
In 1602_6.patch, it looks like this condition is impossible, because
read_console_w() never returns 0x1A at the start of the buffer.

https://bugs.python.org/review/1602/diff/18367/Modules/_io/winconsoleio.c#new...
Modules/_io/winconsoleio.c:691: if (bytes_size < PyBytes_GET_SIZE(bytes)) {
Is this ever likely to happen?

https://bugs.python.org/review/1602/diff/18367/Modules/_io/winconsoleio.c#new...
Modules/_io/winconsoleio.c:692: if (_PyBytes_Resize(&bytes, n * sizeof(wchar_t))
< 0) {
The resize amount doesn’t make sense to me. That would be the number of bytes
that the last wchar_t read took up. Surely it should just be bytes_size.
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7