This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Expose os.device_encoding() at the C level
Type: Stage: resolved
Components: Library (Lib) Versions: Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: brett.cannon Nosy List: benjamin.peterson, brett.cannon, pitrou, python-dev, vstinner
Priority: normal Keywords: patch

Created on 2012-02-28 17:03 by brett.cannon, last changed 2022-04-11 14:57 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
crashing_device_encoding.diff brett.cannon, 2012-02-29 14:43 review
Messages (14)
msg154557 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012-02-28 17:03
The _io module imports the os module purely for the use of os.device_encoding(). That function, though, is defined by posix and thus is already available to C code. To avoid this import dependency it would be better to simply expose the function somehow so that _io can just directly call the function.

Antoine has suggested Python/fileutils.c as a possible place, although I'm personally happy simply exposing the function from posix and making it a METH_O function so that as little code needs to change for _io to use it.
msg154574 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2012-02-28 22:23
If it can help to bootstrap importlib, you can add:

PyObject* _Py_device_encoding(int fd);

And reuse it in posixmodule.c.
msg154593 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012-02-29 02:53
Either that or PyObject *_Py_device_encoding(PyObject *fd) would help since both _io and posixmodule.c will have a PyObject already for the fd.
msg154594 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2012-02-29 03:17
I think fileutils.c is a good place. Then we can kill the import of "os" in io, which would be nice.
msg154595 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012-02-29 03:23
OK, then I will make this happen. People care whether it take an int or a PyObject? That seems to be the only open question.
msg154596 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2012-02-29 03:25
2012/2/28 Brett Cannon <report@bugs.python.org>:
>
> Brett Cannon <brett@python.org> added the comment:
>
> OK, then I will make this happen. People care whether it take an int or a PyObject? That seems to be the only open question.

Probably int if it goes in fileutils.
msg154634 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012-02-29 14:43
Attached is a patch which is trying to do the refactor, but I'm somehow causing a something to be gc'ed when it shouldn't. If anyone spots something obvious let me know, else I will try to debug some more on my way home.
msg154638 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-02-29 15:19
I don't seen any refcounting problem here, but your patch is buggy.
Before patch:

>>> os.device_encoding(0)
'UTF-8'

After patch:

>>> os.device_encoding(0) is None
True

Interestingly, it seems there is no test for os.device_encoding. Perhaps add one to test_os?
msg154639 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2012-02-29 15:25
> your patch is buggy

It should be a missing #include. Maybe this one:

#ifdef HAVE_LANGINFO_H
#include <langinfo.h>
#endif
msg154652 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012-02-29 17:41
Victor's point fixed my issue.

As for a test, I will happily write one, but what can I test? A string for fd 0 and None for fd -1 (or is there some os call I can make to find the largest fd so I can test fd+1)?
msg154654 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-02-29 17:57
> As for a test, I will happily write one, but what can I test? A string
> for fd 0 and None for fd -1 (or is there some os call I can make to
> find the largest fd so I can test fd+1)?

For fd 0, it should be a string if the fd is a tty.
(it should also point to a valid codec)
You can also try fd 42 (same, call isatty() on it).
msg154657 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2012-02-29 18:24
> For fd 0, it should be a string if the fd is a tty.

os.device_encoding() returns None on a non-Windows OS without langinfo support.

locale.nl_langinfo(locale.CODESET) may be used in such test. (to check
for nl_langinfo() support?)
msg154658 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012-02-29 18:38
OK, so if on windows or isatty + nl_langinfo + CODESET, check fd 0 for a returned string that codecs.lookup and find something for. fd 42 (if not a tty) should return None.
msg154668 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2012-02-29 23:31
New changeset c80d9a0bd5a0 by Brett Cannon in branch 'default':
Issue #14153 Create _Py_device_encoding() to prevent _io from having to import
http://hg.python.org/cpython/rev/c80d9a0bd5a0
History
Date User Action Args
2022-04-11 14:57:27adminsetgithub: 58361
2012-02-29 23:32:36brett.cannonsetstatus: open -> closed
assignee: brett.cannon
resolution: fixed
stage: needs patch -> resolved
2012-02-29 23:32:00python-devsetnosy: + python-dev
messages: + msg154668
2012-02-29 18:38:09brett.cannonsetmessages: + msg154658
2012-02-29 18:24:19vstinnersetmessages: + msg154657
2012-02-29 17:57:31pitrousetmessages: + msg154654
2012-02-29 17:41:44brett.cannonsetmessages: + msg154652
2012-02-29 15:25:15vstinnersetmessages: + msg154639
2012-02-29 15:19:26pitrousetmessages: + msg154638
2012-02-29 14:43:55brett.cannonsetfiles: + crashing_device_encoding.diff
keywords: + patch
messages: + msg154634
2012-02-29 03:25:13benjamin.petersonsetmessages: + msg154596
2012-02-29 03:23:32brett.cannonsetmessages: + msg154595
2012-02-29 03:17:46benjamin.petersonsetnosy: + benjamin.peterson
messages: + msg154594
2012-02-29 02:53:07brett.cannonsetmessages: + msg154593
2012-02-28 22:23:08vstinnersetmessages: + msg154574
2012-02-28 19:53:21pitrousetnosy: + vstinner
2012-02-28 17:03:50brett.cannoncreate