[pywin32-bugs] [ pywin32-Patches-1962146 ] win32file.TransmitFile and ConnectEx patch
OLD project page for the Python extensions for Windows
Brought to you by:
mhammond
From: SourceForge.net <no...@so...> - 2008-11-30 23:43:30
|
Patches item #1962146, was opened at 2008-05-12 06:06 Message generated for change (Comment added) made by ionel_mc You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=551956&aid=1962146&group_id=78018 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: None Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: ionel (ionel_mc) Assigned to: Nobody/Anonymous (nobody) Summary: win32file.TransmitFile and ConnectEx patch Initial Comment: sample usage: import win32file, socket, pywintypes s = socket.socket() f = file('somefile','rb') s.connect(('192.168.111.128',10002)) ol = pywintypes.OVERLAPPED() print win32file.TransmitFile(s, win32file._get_osfhandle(f.fileno()), 0, 0, ol, 0, "prepended data", "appended data") length = win32file.GetOverlappedResult(s.fileno(), ol, 1) print length I hope I haven't done the wrong thing but for the sake of simplicity I'm providing a patch the includes the previous ConnectEx patch (hopefully it'll get accepted :) ---------------------------------------------------------------------- >Comment By: ionel (ionel_mc) Date: 2008-12-01 01:43 Message: File Added: test_connectex.py ---------------------------------------------------------------------- Comment By: Mark Hammond (mhammond) Date: 2008-12-01 01:39 Message: Even if you borrowed Python's getaddrinfo, wouldn't you still be in the situation that Roger mentions - getaddrinfo() natively returns a pointer to a linked list, while Python's implementation returns a Python list - which binding in either of the lists are you going to use? It appears you will always use the first? getaddrinfo() returns a list of tuples, which each tuple being fairly simple to unpack (ints and strings IIUC). So if you just accept one of these tuples it should be quite easy to unpack this into an addrinfo struct - just a few lines, and much simpler than either the current code or trying to borrow and adapt Python's implementation. I was about to have a go at modifying your patch to do that (there might be something I don't understand, so that was the best way to find out :), but then noticed that there seems to be no test for ConnectEx() - do you have any code available we can use as a test case for that? Otherwise it will be impossible for me to verify it actually works. Thanks ---------------------------------------------------------------------- Comment By: ionel (ionel_mc) Date: 2008-11-24 13:46 Message: I would rather not do that - too much parameter grunt work. I would rather get the getsockaddrarg implementation from the python's socketmodule and plug it in here. What do you think ? ---------------------------------------------------------------------- Comment By: Roger Upole (rupole) Date: 2008-09-25 15:20 Message: There's another possible issue with the way getaddrinfo is used. It returns a linked list of structures, rather than a single address. This leaves no way to select which sockaddr will be passed to ConnectEx. It might be better to accept a tuple as produced by the builtin socket.getaddrinfo() so that the caller can choose. ---------------------------------------------------------------------- Comment By: ionel (ionel_mc) Date: 2008-09-07 22:40 Message: Logged In: YES user_id=1189761 Originator: YES File Added: win32file.i-9.patch ---------------------------------------------------------------------- Comment By: ionel (ionel_mc) Date: 2008-05-28 02:56 Message: Logged In: YES user_id=1189761 Originator: YES File Added: win32file.i-8.patch ---------------------------------------------------------------------- Comment By: Roger Upole (rupole) Date: 2008-05-28 02:27 Message: Logged In: YES user_id=771074 Originator: NO Regarding the way getaddrinfo is called, it appears that the family, socket type, and protocol are hardcoded. Shouldn't it be using the values from the Python socket object, since these are specified when it's created ? Also, Unicode is accepted for the host, but not the port. This isn't a show stopper, but it would be better to be consistent. ---------------------------------------------------------------------- Comment By: ionel (ionel_mc) Date: 2008-05-13 21:27 Message: Logged In: YES user_id=1189761 Originator: YES File Added: win32file.i-7.patch ---------------------------------------------------------------------- Comment By: ionel (ionel_mc) Date: 2008-05-13 21:16 Message: Logged In: YES user_id=1189761 Originator: YES File Added: win32file.i-6.patch ---------------------------------------------------------------------- Comment By: ionel (ionel_mc) Date: 2008-05-13 18:51 Message: Logged In: YES user_id=1189761 Originator: YES sure, close it. I'll post a message on the list in a few moments. ---------------------------------------------------------------------- Comment By: Mark Hammond (mhammond) Date: 2008-05-13 16:22 Message: Logged In: YES user_id=14198 Originator: NO Sorry for the confusion (and for missing the test!) To be clear, the "problem" is simply my lack of cycles to try and digest what exactly the API allows for versus what your patch does. I'm not suggesting anything you are doing is wrong, just that its not immediately obvious to me (hence my comments re TransmitFile, which a patch for *is* likely to be "obvious" - but as you mentioned you combined them, I didn't look closely). You are correct though that the arg handling is my primary concern, and if you are really keen to get this in ASAP, would you be so kind as to mail the python-win32 list asking for other comments and pointing at this bug? There are a few people there who's opinion I would defer to - and they may have more time at the moment to have a look (I'm really hoping to get build 211 out any day now!) Also, if you are intent on getting these in the same build, should I close the other patch? Thanks! ---------------------------------------------------------------------- Comment By: ionel (ionel_mc) Date: 2008-05-13 13:09 Message: Logged In: YES user_id=1189761 Originator: YES I'd rather have them both in the same build. Also, on the arg handling thing I imagine you didn't like the addrinfo stuff that I did there. It might look like a mess but i have looked in the socketmodule.c from the python dist and there's nothing i can use from there (there is a getsockaddrarg that is used in the connects, it does about the same thing that i do but on a larger _messy_ scale with more cases like unix sockets that don't apply here). Um, I did attach a test_transmitfile.py - should that code really be in test_win32file.py ? ---------------------------------------------------------------------- Comment By: Mark Hammond (mhammond) Date: 2008-05-13 09:29 Message: Logged In: YES user_id=14198 Originator: NO Thanks. Unfortunately, I'm going to need to research the ConnextEx stuff a little more to convince myself the arg handling is reasonable - but it will be accepted one way or another :) However, as TransmitFile is likely to be a much simpler patch, if you supplied a patch for that alone (including tests ideally - see test_win32file.py for stuff you can maybe borrow?) I could get it in much sooner (and it would therefore be possible to make build 211). Otherwise I'll wrap both this and your ConnectEx patches up when I find time to tweak. Thanks again. ---------------------------------------------------------------------- Comment By: ionel (ionel_mc) Date: 2008-05-12 06:06 Message: Logged In: YES user_id=1189761 Originator: YES File Added: test_transmitfile.py ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=551956&aid=1962146&group_id=78018 |