pynetwork / pypcap

pypcap - python libpcap module, forked from code.google.com/p/pypcap
Other
299 stars 75 forks source link

Improve the documentation for __next__() #73

Open JosiahDub opened 6 years ago

JosiahDub commented 6 years ago

After 924dfdeec4d2a045d79f06a80edb96948f0f1184, __next__() continues instead of returning None when timing out. This does not seem like ideal behavior. Am I misinterpreting timeout or misusing the function? Ideal code snippet:

import pcap
p = pcap.pcap(timeout_ms=50)
# Set the filter so that no packets will arrive
p.setfilter("port 55555")
try:
    timestamp, packet = p.__next__()
except TypeError:
    print("__next__() returned None in a reasonable time-frame!")

I modified pcap.pyx to return None and the code snippet worked.

hellais commented 6 years ago

@nuald what are you thoughts on this?

nuald commented 6 years ago

I've already mentioned it here: https://github.com/pynetwork/pypcap/pull/47/files#r116732203 The loop should be infinite - it's the user responsibility to correctly exit the loop. The timeout_ms parameter corresponds to packet buffer timeout (see https://www.tcpdump.org/manpages/pcap.3pcap.html), not to the timeout for exiting the loop if nothing happens. I'd recommend to update the documentation or provide additional timeout parameter to avoid confusion.

hellais commented 6 years ago

Ah right, thanks for reminding me of that. I would say then this ticket should become that of updating the docs to document this as the expected behaviour.

wtdcode commented 4 years ago

@nuald But why not give users a choice to handle the event, like raise TimeoutError or something else?

nuald commented 4 years ago

@wtdcode Sorry, I'm not working on this project nowadays, and only can give some advice: please feel free to modify pcap.pyx for your needs - the packet buffer timeout occurs when pcap_ex_next returns 0 (please see https://www.tcpdump.org/manpages/pcap_next_ex.3pcap.html for the details). Right now in the code there is no special processing for it, and I guess you could add something there instead of just calling continue.