classification
Title: In 2.7.13, _ctypes.LoadLibrary no longer accepts Unicode objects
Type: behavior Stage: resolved
Components: ctypes Versions: Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: martin.panter, python-dev, serhiy.storchaka, yan12125
Priority: normal Keywords: easy (C), patch

Created on 2016-12-27 10:09 by yan12125, last changed 2017-01-12 19:43 by serhiy.storchaka. This issue is now closed.

Files
File name Uploaded Description Edit
issue29082.patch yan12125, 2016-12-31 16:31
issue29082.patch yan12125, 2016-12-31 16:35 Patch version 2
issue29082_3.patch yan12125, 2017-01-12 13:53 Patch version 3
Messages (9)
msg284081 - (view) Author: Chih-Hsuan Yen (yan12125) * Date: 2016-12-27 10:09
In issue27330, there's one more change besides fixing possible memory leaks. In LoadLibrary function of _ctypes: [1]

-    if (!PyArg_ParseTuple(args, "O|O:LoadLibrary", &nameobj, &ignored))
+    if (!PyArg_ParseTuple(args, "S|O:LoadLibrary", &nameobj, &ignored))

Before this change, both bytes and unicode objects are accepted in _ctypes.LoadLibrary() (Unicode objects are implicitly converted to bytes), and after this change only bytes objects are valid.

There are two options:
* Revert the relevant PyArg_ParseTuple.
  It's better to have fewer surprises on 2.7 branch :)
* Document the change.

I prefer the first option as in our project ```from __future__ import unicode_literals``` is used everywhere, and in Python 3 only Unicode objects are acceptable in _ctypes.LoadLibrary().

Downstream report: https://github.com/rg3/youtube-dl/issues/11540

Added the author and the reviewer in issue27330.

[1] e04c054beb53
msg284099 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-12-27 14:04
Good catch Chi Hsuan Yen! This is my mistake, I though PyString_Size() works only with str (as many similar *_Size() functions). Agreed, this change should be reverted. Do you want provide the patch with tests?
msg284111 - (view) Author: Chih-Hsuan Yen (yan12125) * Date: 2016-12-27 16:37
Sorry, but I'm afraid of being unable to test it. I tried to setup a Windows build environment for 2.x but failed. (I've once successfully built 3.x on Windows for issue25939, but things seems different now :(
msg284398 - (view) Author: Chih-Hsuan Yen (yan12125) * Date: 2016-12-31 16:31
I finally get Windows builds working. Here's the patch.
msg284399 - (view) Author: Chih-Hsuan Yen (yan12125) * Date: 2016-12-31 16:35
Oops, Parser/asdl_c.py shouldn't be included. Here's the correct patch
msg284948 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2017-01-08 01:40
Other tests are skipped if libc_name is None, so your assertion is inconsistent.

FTR there are reports open about problems with bootstrap files like asdl_c.py, e.g. Issue 28143 proposing to port that file to Python 3, and Issue 23404 about the future of “make touch”.
msg285327 - (view) Author: Chih-Hsuan Yen (yan12125) * Date: 2017-01-12 13:53
Here's a new patch using fake library names.

And thanks for those related issues about asdl_c.py!
msg285330 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2017-01-12 15:00
New changeset 4ce22d69e134 by Serhiy Storchaka in branch '2.7':
Issue #29082: Fixed loading libraries in ctypes by unicode names on Windows.
https://hg.python.org/cpython/rev/4ce22d69e134
msg285344 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-01-12 19:43
Committed with changed test. Thank you for your patch Chi Hsuan Yen.
History
Date User Action Args
2017-01-17 13:14:35eryksunlinkissue29294 superseder
2017-01-12 19:43:25serhiy.storchakasetstatus: open -> closed
resolution: fixed
messages: + msg285344

stage: patch review -> resolved
2017-01-12 15:00:53python-devsetnosy: + python-dev
messages: + msg285330
2017-01-12 14:16:02serhiy.storchakasetassignee: serhiy.storchaka
2017-01-12 13:53:08yan12125setfiles: + issue29082_3.patch

messages: + msg285327
2017-01-08 01:40:22martin.pantersetmessages: + msg284948
stage: needs patch -> patch review
2016-12-31 16:35:28yan12125setfiles: + issue29082.patch

messages: + msg284399
2016-12-31 16:32:09yan12125setfiles: - LoadLibrary_revert_arg_parsing.patch
2016-12-31 16:31:49yan12125setfiles: + issue29082.patch

messages: + msg284398
2016-12-27 16:37:50yan12125setfiles: + LoadLibrary_revert_arg_parsing.patch
keywords: + patch
messages: + msg284111
2016-12-27 14:04:35serhiy.storchakasetkeywords: + easy (C)

messages: + msg284099
stage: needs patch
2016-12-27 10:09:01yan12125create