Support monitor mode & other features

[gpotter2]

Hi ! This PR fixes #45 and fixes #17

Calling @martinuy for review. I have a working scapy branch (secdev/scapy@490298c) based on this, waiting for it to be merged :)

[Edit: removed « warning » commit]

[gpotter2]

@martinuy @asolino @martingalloar
Hi !
Any news on this PR ?

[martinuy]

Shouldn't strings be casted to "const char*"? (instead of char*) But beyond that, I wonder why was this necessary, as it should be implicitly casted.

Why was pcap_doc changed to be non-static? (const addition looks fine to me)

[gpotter2]

Those additional fixes are fixes for warnings triggered when building the project (Visual Studio Community 2017)

[martinuy]

Hi @gpotter2, thank you very much for your contribution!

Just a few comments:

I've seen that this code is already present in other functions that return a capture handle:

#ifdef WIN32
  pcap_setmintocopy(pt, 0);
#endif

Shouldn't we include it in pcap_create too? I'm not sure, and it's unfortunate that no-comment was added explaining this. Have you tested this on Windows?

Returning ints on set-like functions looks fine to me. Previous idiom (returning None) was hiding information. Ideally, we should also export constants as part of pcapy's API. I.e.: PCAP_ERROR_ACTIVATED.

Names you chose for set-like functions looks fine to me. Previous idiom (having setfunctionname all together) is more confusing.

Your changes on commit 8480721 look fine to me. I suggest merging them after deciding if pcap_setmintocopy is necessary.

[martinuy]

Regarding commit cd5b18c, I understand your point but I'm still not convinced.

Removing static to pcap_doc makes me think that the symbol is exported and is relevant out of the object, when it looks like an internal variable. Can you show me this compiler warning?

In regard to casting strings, the problem is the following: a string (like "pcapy.PcapError") is of const char* type. However, PyErr_NewException receives char* as its first parameter [1]. That's why the compiler is raising a warning. We have 2 options here: 1) PyErr_NewException wants to modify the string (and that's why const was not specified) or 2) const was simply ommitted (either because it was missed or because some compiler compiling a non C89+ C was not okay with this). I think option 2) is the most likely one. Compilers will put literal strings in read-only memory. So, casting to non-const makes no sense in run time: as soon as the string is modified, a segmentation fault will occur. My suggestion is to live with the warning. Sorry, I don't like it either, but the cast in the code looks even worse to me.

[1] - https://docs.python.org/2/c-api/exceptions.html

[gpotter2]

I’ve removed the warning commit.

FTR: pcap_setmintocopy call on the pointer makes winpcap returns the packet as soon as it receives them, and not buffer them (default is that a size of 16000 is required to be in the buffer before it gets returned). Windows only

[gpotter2]

After a small investigation, pcap_setmintocopy is something we want to include.

I've added a small explanation (it's windows only) in the code.

@martinuy That should be it for this PR :) Please merge as soon as it seems ready to you

[Edit: mis-clicked on close]

[gpotter2]

Also, it would be great if after merging this PR, you could regenerate the HTML manual

[martinuy]

@gpotter2 , awesome! You've done a great job :-)

[martinuy]

@gpotter2 , regarding HTML regeneration, have you updated the XML? (if I remember correctly, we used to do that manually)

[gpotter2]

Regerenerated the manual in #50