Python 3 compatibility, timeouts and exception handling have been fixed.

[nuald]

timeout_ms parameter hasn't been working properly for both __next__() and loop() methods, it's fixed now. dispatcher() didn't process exceptions correctly (see the previous comments in test.py), it's fixed too.

Additional changes: the code have been regenerated with the new Cython, almost all warnings (few left for Windows) have been fixed.

Verified: used the README code and both test scripts in tests/ dir. Platforms: Windows 10 (py2.7), macOS Sierra (py 2.7), CentOS 6 with Python 2.6.

[hellais]

Thanks so much for contributing to pypcap!

I think it would be best to defer merging this to post 1.1.6. I see there are a fair amount of changes in this PR and I will have to review it with some care.

[nuald]

No problems. Frankly, I think it's better to merge Py3 support (#46, not sure of its status though) first, after that I can update the code to work correctly with both Python 2 and 3 versions.

[hellais]

Frankly, I think it's better to merge Py3 support (#46, not sure of its status though) first, after that I can update the code to work correctly with both Python 2 and 3 versions.

yes that makes sense.

If you have experience with making cythong bindings supporting python2 and 3 feedback on #46 would be greatly appreciated.

[nuald]

Python 3 compatibility has been added. However, looks like Python 2.6 have some issues (not the library itself, but dpkt won't work with memoryviews due to python 2.6 struct.unpack limited functionality).

I don't recommend to merge yet, but please verify on all available platforms, I'll do it too. Python 2.6 issue looks like minor, because it's not officially supported anyway.

[nuald]

Verified (only 64-bit platforms):

• CentOS 6.6: Python 2.6.6
• Debian Jessie: Python 2.7.9, Python 3.4.2, PyPy 2.4.0 (Python 2.7.8)
• macOS Sierra: Python 2.7.13, Python 3.6.1, PyPy 5.7.1 (Python 2.7.13)
• Windows 10: Python 2.7.13, Python 3.6.1, PyPy 5.4.1 (Python 2.7.10)

[nuald]

Please note that I've finalized Python 3 testing, and confident that the change doesn't break Python 2 code (it may have some Python 3 compatibility issues though, but until it's released I doubt we can find all cases). Please merge when it's possible, so I can continue with the pending tasks from my side (Npcap native support and bdist_wheel packages).

[hellais]

Great work. Thank you so much for this. I will see if I can take a look at reviewing this in some hours.

[hellais]

I am so sorry for taking so long to look at this. I am bit worried of merging all these changes without doing first a bugfix (what is in the 1.1.6 branch). So I am going to first merge that, cut 1.1.6 and then proceed with reviewing carefully this branch and making a 1.2.0, does that seem reasonable?

[nuald]

Sure, no problem. If you need any help, please let me know.

[hellais]

First, thanks so much for hacking on this, it's super useful and great work! 👍

Secondly, I sprinkled some comments on the PR of things that should either be addressed in this PR or questions I have.

In general I think this is good, the main things that I would like to have happen before we merge it is:

• [ ] Ensure that it makes sense to break the API by returning unicode in some method call and if so document this breaking change in the changelog
• [ ] Fix the bikesheddy tab vs space issue in pcap_ex.c
• [ ] Address question regarding the pcap iterator and if it is fixing a known bug mention it in the changelog

[nuald]

About unicode. The goal was to have the same API code for both Python 2 and 3 (e.g. the test code shouldn't be changed). It is possible to add Python 2 checks and returns str instead of unicode, however in that case we need to use CPython API for the strings manipulations and do not rely on Cython auto-generated code. Python 2 code requiring actual str type is not so common nowadays as a lot of developers writing cross-version code, so please provide me the example where it's actually happening, so I can add tests for this use-case.

pcap_ex.c formatting. I'd like to suggest reformatting it after the merge.

pcap iterator. It may be related to #49, I have to take a look. The code has been modified for the consistency, so the iterator works the same way as the loop() method and timeouts relate directly to the libpcap function:

packet buffer timeout If, when capturing, packets are delivered as soon as they arrive, the application capturing the packets will be woken up for each packet as it arrives, and might have to make one or more calls to the operating system to fetch each packet. If, instead, packets are not delivered as soon as they arrive, but are delivered after a short delay (called a "packet buffer timeout"), more than one packet can be accumulated before the packets are delivered, so that a single wakeup would be done for multiple packets, and each set of calls made to the operating system would supply multiple packets, rather than a single packet. This reduces the per-packet CPU overhead if packets are arriving at a high rate, increasing the number of packets per second that can be captured. The packet buffer timeout is required so that an application won't wait for the operating system's capture buffer to fill up before packets are delivered; if packets are arriving slowly, that wait could take an arbitrarily long period of time.

The last sentence is exactly the problem I've faced with npcap on Windows 10 - without timeout I had to wait until the capture buffer is filled and couldn't see the traffic in real time.

To avoid big pull requests, I'd like to suggest merging this PR into some branch (like python3-support), and I'll continue to work with that branch. Also end users will be able to checkout the branch and verify the functionality they need.

[hellais]

I'd like to suggest merging this PR into some branch (like python3-support)

Yes I like this idea. This way we can also encourage people to do some testing of it before we make a release.

[hellais]

I also agree on the other points, so I am going to merge this into feature/python3. Let's work in there adding changelog entries and preparing this for landing into master and into a new release.

[guyharris]

The last sentence is exactly the problem I've faced with npcap on Windows 10 - without timeout I had to wait until the capture buffer is filled and couldn't see the traffic in real time.

A timeout of 0

1. is not guaranteed to have the same behavior on all platforms;
2. in practice, will probably block until the buffer fills up on most platforms, thus delaying for an indefinite and unbounded period of time;

so it's not recommended.