[issue17931] PyLong_FromPid() is not correctly defined on Windows 64-bit
Changes by Richard Oudkerk shibt...@gmail.com: -- status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17931 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17931] PyLong_FromPid() is not correctly defined on Windows 64-bit
Richard Oudkerk added the comment: pid_t is HANDLE on Windows, which is a pointer. I think this is wrong. The signature of getpid() is int _getpid(void); so pid_t should be equivalent to int. The complication is that the return values of spawn*() etc are process handles (cast to intptr_t), not pids: intptr_t _spawnv(int mode, const char *cmdname, const char *const *argv); See http://msdn.microsoft.com/en-us/library/t2y34y40%28v=vs.100%29.aspx http://msdn.microsoft.com/en-us/library/7zt1y878%28v=vs.80%29.aspx -- nosy: +sbt status: closed - open ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17931 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17931] PyLong_FromPid() is not correctly defined on Windows 64-bit
STINNER Victor added the comment: The complication is that the return values of spawn*() etc are process handles (cast to intptr_t), not pids: I opened this issue because of a compiler warning in os.waitpid(). On Windows, the C function _cwait() is used and it expects a intptr_t (handle to the process to wait on) and returns a intptr_t (returns the handle of the specified process) But os.waitpid() uses PyLong_FromPid() to convert the C intptr_t to a Python int, which may loose most significant bits (sizeof(void*) sizeof(int) on Windows x64). I see that PC/pyconfig.h defines typedef int pid_t;. Because intptr_t is bigger than int, using intptr_t is compatible with pid_t, at least for PyLong_FromPid(). It may be a problem in the other direction: PyLong_AsPid(). Python code using Windows HANDLE is not consistent. Sometimes, intptr_t is used, sometimes long is used. Examples: - msvcrt.open_osfhandle() parses an handle with l (long) format, whereas it should uses a wider type. - msvcrt.get_osfhandle() uses PyLong_FromVoidPtr() to convert an C Py_intptr_t to a Python int, with the following comment: /* technically 'handle' is not a pointer, but a integer as large as a pointer, Python's *VoidPtr interface is the most appropriate here */ -- @sbt: Would you like to have a strict separation between UNIX-like pid (pid_t) and Windows process identifier (HANDLE)? Can we consider that HANDLE is a pointer? Or can we consider that HANDLE is intptr_t or intmax_t? See the issue #17870 which proposes an API to handle intmax_t. If we want to be more explicit, we should add functions to handle the C HANDLE type. Example: - PyLong_FromHandle(): C HANDLE to Python int - PyLong_AsHandle(): Python int to C HANDLE - P format (or another letter): PyArg_Parse*() format -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17931 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17931] PyLong_FromPid() is not correctly defined on Windows 64-bit
Richard Oudkerk added the comment: @sbt: Would you like to have a strict separation between UNIX-like pid (pid_t) and Windows process identifier (HANDLE)? Yes. And would I certainly like SIZEOF_PID_T == sizeof(pid_t) ;-) Note that _winapi takes the policy of treating HANDLE as an unsigned quantity (as PyLong_*VoidPtr() does for pointers). I am not sure if signed or unsigned is better, but I lean towards unsigned. It is easy enough to cast to intptr_t if we need to. I think it is enough to treat HANDLE as void*, but adding PyLong_*Handle() is easy enough. There does not seem to be a format character for void* (or size_t), and adding one would be useful. Or maybe rather than adding ever more format characters which are aliases for old ones, we could just create macros like #define PY_PARSE_INT i #define PY_PARSE_UINTPTR_T K #define PY_PARSE_VOID_PTR PY_PARSE_UINTPTR_T #define PY_PARSE_HANDLE PY_PARSE_UINTPTR_T #define PY_PARSE_PID_T PY_PARSE_INT -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17931 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17931] PyLong_FromPid() is not correctly defined on Windows 64-bit
Richard Oudkerk added the comment: I see _Py_PARSE_PID already exists but no others ... -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17931 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17931] PyLong_FromPid() is not correctly defined on Windows 64-bit
Richard Oudkerk added the comment: Attached is a patch that adds _Py_PARSE_INTPTR and _Py_PARSE_UINTPTR to Include/longobject.h. It also uses _Py_PARSE_INTPTR in Modules/posixmodule.c and PC/msvcrtmodule.c and removes the definition for SIZEOF_PID. -- Added file: http://bugs.python.org/file30472/py_parse_intptr.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17931 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17931] PyLong_FromPid() is not correctly defined on Windows 64-bit
STINNER Victor added the comment: -/* The size of `pid_t' (HANDLE). */ -#define SIZEOF_PID_T SIZEOF_VOID_P I would prefer to have SIZEOF_PID_T defined: #define SIZEOF_PID_T SIZEOF_INT win32_kill() uses DWORD type, not pid_t. Except these nits, your patch looks good and is more correct than my patch. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17931 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17931] PyLong_FromPid() is not correctly defined on Windows 64-bit
Roundup Robot added the comment: New changeset 0410bf251e10 by Richard Oudkerk in branch 'default': Issue #17931: Resolve confusion on Windows between pids and process handles. http://hg.python.org/cpython/rev/0410bf251e10 -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17931 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17931] PyLong_FromPid() is not correctly defined on Windows 64-bit
Roundup Robot added the comment: New changeset 2298bcba6ec9 by Victor Stinner in branch 'default': Close #17931: Fix PyLong_FromPid() on Windows 64-bit: processes are identified http://hg.python.org/cpython/rev/2298bcba6ec9 -- nosy: +python-dev resolution: - fixed stage: - committed/rejected status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17931 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17931] PyLong_FromPid() is not correctly defined on Windows 64-bit
Changes by Mark Dickinson dicki...@gmail.com: -- nosy: +mark.dickinson ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17931 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17931] PyLong_FromPid() is not correctly defined on Windows 64-bit
STINNER Victor added the comment: @Antoine (author of the commit fixing #1983): any opinion? -- nosy: +pitrou ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17931 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17931] PyLong_FromPid() is not correctly defined on Windows 64-bit
Antoine Pitrou added the comment: Sounds fine to me, but perhaps better test the patch before committing? (or wait for the buildbots to crash) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17931 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17931] PyLong_FromPid() is not correctly defined on Windows 64-bit
STINNER Victor added the comment: Oh, I just noticed the following check in pyport.h: #if SIZEOF_PID_T SIZEOF_LONG # error Python doesn't support sizeof(pid_t) sizeof(long) #endif I don't understand this test, longobject.h contains: #elif defined(SIZEOF_LONG_LONG) SIZEOF_PID_T == SIZEOF_LONG_LONG #define _Py_PARSE_PID L #define PyLong_FromPid PyLong_FromLongLong #define PyLong_AsPid PyLong_AsLongLong I suppose that this path was never tested before. Here is a new patch removing the check from pyport.h, longobject.h already fails if SIZEOF_PID_T value is not supported: #else #error sizeof(pid_t) is neither sizeof(int), sizeof(long) or sizeof(long long) #endif /* SIZEOF_PID_T */ -- Added file: http://bugs.python.org/file30289/win_sizeof_pid_t-2.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17931 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17931] PyLong_FromPid() is not correctly defined on Windows 64-bit
New submission from STINNER Victor: The issue #1983 was not fixed on Windows: pid_t is HANDLE on Windows, which is a pointer. SIZEOF_PID_T is not defined in PC/pyconfig.h and so longobject.h takes the default implementation (use C long type): /* Issue #1983: pid_t can be longer than a C long on some systems */ #if !defined(SIZEOF_PID_T) || SIZEOF_PID_T == SIZEOF_INT #define _Py_PARSE_PID i #define PyLong_FromPid PyLong_FromLong #define PyLong_AsPid PyLong_AsLong #elif SIZEOF_PID_T == SIZEOF_LONG ... The consequence is a compiler warning: ..\Modules\posixmodule.c(6603): warning C4244: 'function' : conversion from 'Py_intptr_t' to 'long', possible loss of data [C:\buildbot.python.org\3.x.kloth-win64\build\PCbuild\pythoncore.vcxproj] It would be safer to define SIZEOF_PID_T on Windows: #define SIZEOF_PID_T SIZEOF_VOID_P I didn't test attached patch on Windows. Python 3.2 is affected, but I don't think that the issue is important enough to touch this branch (which now only accept security fixes). See also issue #17870. -- components: Windows files: win_sizeof_pid_t.patch keywords: patch messages: 188688 nosy: haypo, serhiy.storchaka, tim.golden priority: normal severity: normal status: open title: PyLong_FromPid() is not correctly defined on Windows 64-bit versions: Python 2.7, Python 3.2, Python 3.3, Python 3.4 Added file: http://bugs.python.org/file30170/win_sizeof_pid_t.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17931 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17931] PyLong_FromPid() is not correctly defined on Windows 64-bit
Changes by Antoine Pitrou pit...@free.fr: -- nosy: +brian.curtin ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17931 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
