homepage

subprocess and multiprocessing both have their own private modules for wrappers of win32 functions: Modules/_multiprocessing/win32_functions.c and PC/_subprocess.c.
It would be nice to group them in a common module (_win32?) that could be used throughout the stdlib.

+1

Agreed; I'm not personally the windows expert that should handle that consolidation though.

Big +1. I'll work up a patch.

Here's a patch replacing Modules/_multiprocessing/win32_functions.c and PC/_subprocess.c with a common PC/_windows.c. There's not much to the patch despite its size -- it just shuffles around the C code and does a few renames in the appropriate Python modules. All tests pass.
I left the copyright notice from PC/_subprocess.c at the top. No idea if that needs to stay or not.

Two high-level remarks about the patch:
- IMO there is no reason to put _windows.c in the PC directory. After all, there is no such distinction for posix-specific modules.
- wxPython already has a submodule named _windows.py. I wonder if this will cause problems.

For the first point, I just put it there since other Windows-only modules already exist there. _subprocess did, msvcrt and winreg currently do, and there's a few other Windows-only things in there. It's not a big deal, so I can move it into Modules if we want -- winreg and msvcrt should probably get moved as well (in another issue).
As for the name clash, I could shorten it to _win, but I'd rather not name it _win32. Microsoft got away from calling it the "Win32 API" and instead say "Windows API" now since it also covers 64-bit. It's just an internal name so I won't fight too hard on this.

I agree with Amaury that it would be better in Modules. In my experience, code that is in PC/ is a pain to discover.
A couple of nits about the patch:
- the functions in the PyMethodDef array could be sorted alphabetically
- the defint() macro isn't necessary, PyModule_AddIntMacro() should do the trick
- the "_win_handle_object" thing seems misguided, I would vote to remove it, and call CloseHandle() from Python code instead

PS: I don't think there's a problem with the "_windows" name, as long as wxPython doesn't depend on a *toplevel* module named "_windows".

I think there are some issues with the treatment of the DWORD type. (DWORD is a typedef for unsigned long.)
_subprocess always treats them as signed, whereas _multiprocessing treats them (correctly) as unsigned. _windows does a mixture: functions from _subprocess parse DWORD arguments as signed ("i"), functions from _multiprocessing parse DWORD arguments as unsigned ("k"), and the constants are signed.
So in _windows the constants GENERIC_READ, NMPWAIT_WAIT_FOREVER and INFINITE will be negative. I think this will potentially cause errors from PyArg_ParseTuple() when used as arguments to functions from _multiprocessing.
I think it is also rather confusing that some functions (eg CreatePipe()) return handles using a wrapper type which closes on garbage collection, while others (eg CreateNamedPipe()) return handles as plain integers.
(The code also needs updating because quite a few functions have since been added to _multiprocessing.win32.)

Attached is an up to date patch.
* code has been moved to Modules/_windows.c
* DWORD is uniformly treated as unsigned
* _subprocess's handle wrapper type has been removed (although 
 subprocess.py still uses a Python implemented handle wrapper type)
I'm not familiar with Visual Studio. I ended up copying _socket.vcproj 
to _windows.vcproj and editing it by hand. I also edited 
_multiprocessing.vcproj and pythoncore.vcproj by hand.

I don't think we need the vcproj file, unless I missed something.

> I don't think we need the vcproj file, unless I missed something.
_multiprocessing.win32 currently wraps closesocket(), send() and recv() so it needs to link against ws2_32.lib.
I don't know how to make _windows link against ws2_32.lib without adding a vcproj file for _windows unless we make pythoncore depend on ws2_32.lib. I presume this is why _socket and _select have their own vcproj files.
Maybe the socket functions could be moved directly to the top level of _multiprocessing instead since they are not really win32 functions. (And I suppose if that does not happen then _multiprocessing should also stop linking against ws2_32.lib.)
BTW why does _select link against wsock32.lib instead of ws2_32.lib?

It shouldn't. I noticed this and fixed this at CCP a while back but I wasn't in Python Committer mode at the time. _select needs fixing.

(fixed wsock32.lib in revision ab0aff639cfb)

New patch. Compared to the previous one:
* socket functions have been moved from _windows to _multiprocessing
* _windows.vcpoj has been removed (so _windows is part of pythoncore.vcproj)
* no changes to pcbuild.sln needed
* removed reference to 'win32_functions.c' in setup.py
(I am not sure whether/how setup.py is used on Windows.)
 Lib/multiprocessing/connection.py | 124 +-
 Lib/multiprocessing/forking.py | 31 +-
 Lib/multiprocessing/heap.py | 6 +-
 Lib/multiprocessing/reduction.py | 6 +-
 Lib/subprocess.py | 104 +-
 Lib/test/test_multiprocessing.py | 2 +-
 Modules/_multiprocessing/multiprocessing.c | 83 +-
 Modules/_multiprocessing/win32_functions.c | 823 ----------------
 Modules/_windows.c | 1337 +++++++++++++++++++++++++++
 PC/_subprocess.c | 697 --------------
 PC/config.c | 6 +-
 PCbuild/_multiprocessing.vcproj | 4 -
 PCbuild/pythoncore.vcproj | 8 +-
 setup.py | 1 -
 14 files changed, 1568 insertions(+), 1664 deletions(-)

> New patch. Compared to the previous one:
> 
> * socket functions have been moved from _windows to _multiprocessing
> * _windows.vcpoj has been removed (so _windows is part of pythoncore.vcproj)
> * no changes to pcbuild.sln needed
> * removed reference to 'win32_functions.c' in setup.py
I think the module would be better named _win32, since that's the name
of the API (like POSIX under Unix).
Also, it seems there are a couple of naming inconsistencies renaming
(e.g. the overlapped wrapper is named
"_multiprocessing.win32.Overlapped")
Otherwise, I guess it's ok.
> (I am not sure whether/how setup.py is used on Windows.)
Neither do I. It may be used under mingw or cygwin, but we don't
officially support these.

> I think the module would be better named _win32, since that's the name
> of the API (like POSIX under Unix).
Changed in new patch.
> Also, it seems there are a couple of naming inconsistencies renaming
> (e.g. the overlapped wrapper is named "_multiprocessing.win32.Overlapped")
I've fixed that one (and changed the initial comment at the beginning of _win32.c), but I can't see any other.
I also removed a duplicate of getulong().

pythoncore.vcproj)
> > * no changes to pcbuild.sln needed
> > * removed reference to 'win32_functions.c' in setup.py
>
> I think the module would be better named _win32, since that's the name
> of the API (like POSIX under Unix).
While there are many references to it being called Win32 API around the
web, at some point it became the Windows API.

>at some point it became the Windows API.
You are right: http://en.wikipedia.org/wiki/Windows_API
How about _windowsapi or _winapi then, to ensure there are no clashes?

> How about _windowsapi or _winapi then, to ensure there are no clashes?
I don't have any strong feelings, but I would prefer _winapi.

> > How about _windowsapi or _winapi then, to ensure there are no clashes?
> 
> I don't have any strong feelings, but I would prefer _winapi.
Ditto here.

s/_win32/_winapi/g

> sbt <shibturn@gmail.com> added the comment:
> 
> s/_win32/_winapi/g
Overlapped's naming is still lagging behind :-)
Other than that, a comment:
+ def Close(self):
+ if not self.closed:
+ self.closed = True
+ _winapi.CloseHandle(self)
Since Close() can be called at shutdown (through __del__), it should
probably cache its references to globals (because of the unpredictable
order of module cleanup), like this:
+ def Close(self, CloseHandle=_winapi.CloseHandle):
+ if not self.closed:
+ self.closed = True
+ CloseHandle(self)
Otherwise, looks good (I haven't tested).

> Overlapped's naming is still lagging behind :-)
Argh. And a string in winapi_module too.
Yet another patch.

New changeset f3a27d11101a by Antoine Pitrou in branch 'default':
Issue #11750: The Windows API functions scattered in the _subprocess and
http://hg.python.org/cpython/rev/f3a27d11101a 

Thanks a lot for doing this! Patch now committed to 3.3 (after testing under Windows 7 64 bits).